Simplify markdown table generation in magic command - %ai list #1251
Conversation
keerthi-swarna
added
good first issue
There was a problem hiding this comment.
@keerthi-swarna Great work! I left some feedback for you below.
One other issue I noticed is that the py-markdown-table dependency is not available on Conda Forge. Conda Forge is an alternative package registry that allows users to install jupyter-ai using conda, mamba, or micromamba. We can't merge this PR until that is available on Conda Forge, as otherwise installing Jupyter AI from Conda Forge would be broken.
Creating a new Conda Forge package from an existing PyPI package will be a challenge. I would love for you to own this because it is a great opportunity to develop a unique & useful skill. However, I also understand if you need more time before tackling this. Let's talk about this in our next 1:1.
| "<ul><li> For Cross-Region Inference use the appropriate `Inference profile ID` (Model ID with a region prefix, e.g., `us.meta.llama3-2-11b-instruct-v1:0`). See the [inference profiles documentation](https://docs.aws.amazon.com/bedrock/latest/userguide/inference-profiles-support.html).</li></ul>" | ||
| "<ul><li> For custom/provisioned models, specify the model ARN (Amazon Resource Name) as the model ID. For more information, see the [Amazon Bedrock model IDs documentation](https://docs.aws.amazon.com/bedrock/latest/userguide/model-ids.html).</li></ul>" |
There was a problem hiding this comment.
Can you explain the motivation behind this change? This doesn't seem related to the other changes.
| @@ -30,6 +30,7 @@ dependencies = [ | |||
| "typing_extensions>=4.5.0", | |||
| "click~=8.0", | |||
| "jsonpath-ng>=1.5.3,<2", | |||
| "py_markdown_table>=1.3.0" | |||
There was a problem hiding this comment.
We should have a version ceiling in each of our dependencies. Without a version ceiling, users may accidentally install py_markdown_table==2.* if a v2 version of that package were to be released.
Since upgrading a major version generally introduces breaking changes, we should prevent this by adding a version ceiling:
| "py_markdown_table>=1.3.0" | |
| "py_markdown_table>=1.3.0,<2.0.0" |
| providers_info_markdown_table | ||
| + alias_markdown_table_header | ||
| + alias_markdown_table | ||
| ) | ||
|
|
||
| def _ai_list_command_text(self, single_provider=None): |
There was a problem hiding this comment.
Can we return the same string in both the Markdown & text views? The text view is only seen when running ipython from the command line. You can see the different output by running this in your terminal:
% ipython
Python 3.11.11 | packaged by conda-forge | (main, Dec 5 2024, 14:21:42) [Clang 18.1.8 ]
Type 'copyright', 'credits' or 'license' for more information
IPython 8.30.0 -- An enhanced Interactive Python. Type '?' for help.
In [1]: %reload_ext jupyter_ai_magics
In [2]: %ai list
Right now, when you run %ai list from the command line, it is calling _ai_list_command_text(), which produces something different. It seems weird that we have two different implementations for the same command. Can you explore showing the Markdown tables when %ai list is called from ipython directly?
To get started, the handle_list() method should be changed to:
def handle_list(self, args: ListArgs):
markdown = self._ai_list_command_markdown(args.provider_id)
return TextOrMarkdown(
markdown,
markdown,
)
Issue Reference: Issue-118
Description
Refactored the code responsible for generating markdown tables by replacing custom imperative logic with the py-markdown-table library.
Changes
_ai_list_command_markdownmethod inmagics.pyto generate markdown tables using the py-markdown-table library._ai_env_status_for_provider_markdownmethod to return theenv_variablealongside its status in emoji form, replacing the previous approach where the emoji was appended to theenv_variablestring.py-markdown-tablelibrary as a dependency inpyproject.toml.aws.pyfile to address an issue where markdown was not rendering correctly within a markdown table cell. To resolve this, markdown syntax was replaced with HTML tags to ensure proper rendering.Testing