-
Notifications
You must be signed in to change notification settings - Fork 5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support pandas #10
Support pandas #10
Conversation
@@ -37,7 +37,7 @@ def load_ipython_extension(ip): | |||
plugin = SqlPlugin(shell=ip, config=ip.config) | |||
configure_prompt(plugin) | |||
_loaded = True | |||
ipydb_help() | |||
print('\nLoad ipydb extension done, type %ipydb_help for documentation of ipydb') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, but, could you make the messages the first line of the previous help message:
u"Welcome to ipydb %s!" % __version__
I had a play around in notebook on this pull request and it seems pretty cool. In the end notebook queries should do pandas by default but that could be treated as a separate thing (i.e. don't do console things when in notebook). Nice work @litaotao |
@krockode Good point. I think that we should detect if we are running in an ipython-notebook and return the pandas dataframe by default. I still see a use for the -P option, or course, users might want to return a dataframe while running in the ipython-console to take advantage of pandas functionality there. One further point: pandas (which requires numpy) is a very heavy dependency / download and incurs a large compile time (lots of c-extension code) if not using wheel packages. I think we should make pandas an optional dependency, similar to the way that ipython-notebook works. i.e. To do this, we add the following to setup.py: extras_require: {
'notebook': ['pandas>=0.16.2']
} And also wrap the import statements that use pandas. Something like this: _has_pandas = False
try:
import pandas
_has_pandas = True
except ImportError:
pass And later (for e.g. in magic.py line 205): if args.pandas and not _has_pandas:
print("Warning: Pandas support not installed. Please use `pip install 'ipydb[notebook]'` "
"to add support for pandas dataframes in ipydb.") |
My guess is that it's because of the "This branch has conflicts that must be resolved" so it doesn't show the travis stuff?? Looks like @jaysw updated some tests or something a couple of days ago which isn't in your branch. Try merging jaysw master and see if that fixes it. |
@litaotao, thanks for the updates! Could you please merge master as @krockode suggests. Also please note that I had to clamp the version of mock that I am using to an older version (due to some API change in newer versions) Be sure to downgrade mock to the version listed in setup.py in master. That will fix the test run. I'll tidy up and get things running with newer mock later on. Also yes please try for running with pandas by default in ipython notebook for this PR. Thanks again, much appreciated! |
I'm guessing that's going to get convoluted really quickly. Here is a relavent stackoverflow. I think the main issue is that you could have multiple clients connecting to the ipython kernel. Some could be consoles and some could be notebooks. It may or may not be worth tackling that issue in this pull request though. |
Is there anything wrong with this PR. |
Merged. Thanks for your contribution, litaotao. Apologies for the delay. |
@jaysw ha, I was wondering is there anything need update about this PR, cause I would like to use |
Nope, perfect. Thanks litaotao. I am adding you to the authors list for ipydb, how would you like to be known?
Is that okay? |
thanks a lot, sure, that's fine for me. |
Done, thanks again. |
ha~, my pleasure |
update
thanks