Skip to content
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

only import pandas when export_pandas gets called #53

Merged
merged 1 commit into from
Nov 26, 2016

Conversation

dakra
Copy link
Contributor

@dakra dakra commented May 21, 2016

I guess in most production and testing environments pandas is not installed,
so every time you import pydruid anything you get an (annoying) warning message
printed even though you never use the query.export_pandas function.

This pull moves the import into the function so you only get an error when you
actually call export_pandas.

@nishantmonu51
Copy link
Member

👍

try:
import pandas
except ImportError:
print('Warning: unable to import Pandas. The export_pandas method will not work.')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should change this to be an error instead of a warning in this case, since we are raising an exception

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right.
should I just remove the try-except then?
Think the normal python ImportError: No module names 'pandas' is good enough.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I just moved the import pandas statement from the header to the function.
I think the normal ImportError message is enough and you don't get the
annoying print warning when you don't have pandas installed and don't want to use
the function.

@erikdubbelboer
Copy link
Contributor

Is there any plan om merging this soon? Seems like a no-brainer to me.

@fjy
Copy link
Member

fjy commented Nov 26, 2016

👍

@fjy fjy merged commit c64d0c5 into druid-io:master Nov 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants