provide some IPython implementations #956
Conversation
Codecov Report
@@ Coverage Diff @@
## main #956 +/- ##
==========================================
- Coverage 98.59% 98.14% -0.45%
==========================================
Files 50 50
Lines 8962 9013 +51
Branches 1598 1607 +9
==========================================
+ Hits 8836 8846 +10
- Misses 49 90 +41
Partials 77 77
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| def _ipython_key_completions_(self) -> List[str]: | ||
| """Provide custom key completions for the DataFrame in IPython or Jupyter Notebook. | ||
|
|
There was a problem hiding this comment.
a simple unit test can be added here
| f"There are {count} rows. Showing only {rows_limit}. Change DataFrame.__rows_limit value to display more rows" | ||
| ) |
There was a problem hiding this comment.
| f"There are {count} rows. Showing only {rows_limit}. Change DataFrame.__rows_limit value to display more rows" | |
| ) | |
| f"There are {count} rows. Showing only {rows_limit}. Change DataFrame.__rows value to display more rows" | |
| ) |
the class variable is called __rows
| import IPython | ||
| import pandas as pd |
There was a problem hiding this comment.
Can you elaborate on when this function is called and what happens when pandas is not available
For a safe way to import pandas, you can follow
we should have dependencies well documented when users want to use IPython magics. My guess is that IPython is always installed when running inside any IPython notebook.
There was a problem hiding this comment.
Yes IPython will always be available in jupyter. For the pandas... yes we should check for that. I see know that there is an existing mechanism for checking pandas presence. I think I can use that and if not just use df.show
There was a problem hiding this comment.
I guess this function is only called by IPython notebook
There was a problem hiding this comment.
yes, it is a ipython method: https://ipython.readthedocs.io/en/stable/config/integrating.html
| # class property to control the number of rows to diplay | ||
| __rows = 50 |
There was a problem hiding this comment.
Is this naming convention followed by ipython variables? I think there should be a better way to set this property because users should not be accessing variables that start with _.
There was a problem hiding this comment.
No, I was just thinking on a way to expose a mechanism for setting the number of rows to display by default. Maybe a more explicit value in the session object for example will be better.
| def register_sql_magic(): | ||
| try: |
There was a problem hiding this comment.
can you describe what is the purpose of this function or point me to some documentation I can follow
There was a problem hiding this comment.
the purpose of this function is that when an import is done it will run init.py and call register_sql_magic. This will function will try to gather the ipython environment and if found it will register the sql magic
| except Exception as ex: | ||
| return str(ex) |
There was a problem hiding this comment.
how is the exception displayed on the client side. Is it just a cell with error message?
There was a problem hiding this comment.
it will just displayed in the cell output. Would you like to edit the traceback?
| user_ns = IPython.get_ipython().user_ns | ||
| if "session" in user_ns: | ||
| session = user_ns["session"] |
There was a problem hiding this comment.
how does user_ns get populated with objects? how do we know that snowflake.snowpark.Session will always be mapped to "session"
There was a problem hiding this comment.
We don't know before hand, that why we check and if it is not there we notify the user. We can iterate thru all the objects until we find a session object as well.
There was a problem hiding this comment.
we would either need to document that session object must be named session and no other object must be named session, else these changes won't work
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-871864: Better IPython integration #955
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
This PR extends some classes adding IPython extensions