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

Update read_table to pandas 0.42 read_csv #353

Closed
wants to merge 1 commit into from

Conversation

deculler
Copy link
Contributor

@deculler deculler commented Jun 7, 2019

[ X] Wrote test for feature
[ ] Added changes in the Changelog section in README.md
[ ] Bumped version number (delete if unneeded)

Changes proposed:
Updates read_table function to use pandas.read_csv rather than pandas.read_table, which was deprecated 0.24

@adnanhemani
Copy link
Member

As I listed in #352 in more detail, I think it would be okay to not make this change (I would actually recommend against for readability purposes only) - it shouldn't affect our deployment on datahub and this deprecation looks like it'll be reversed in the next version of Pandas. If we still want to make this change, I think it should still be okay, though.

@adnanhemani
Copy link
Member

Related to this, perhaps, we should lock down a fixed Pandas version number in our requirements.txt? I'm noticing that Pandas isn't even listed as a dependency that we rely on in our requirements.txt file - but as our library relies on it to import tables, we probably should have it listed.

@davidwagner
Copy link
Member

@adnanhemani, John says he doesn't want to list list pandas in the requirements.txt for datascience, as it causes surprising results when newcomers install datascience via pip. We could list a version in the datahub config but maybe it's not needed.

@deculler
Copy link
Contributor Author

deculler commented Jun 9, 2019 via email

@adnanhemani adnanhemani mentioned this pull request Jun 11, 2019
@davidwagner
Copy link
Member

I've merged #362, which is based on this, so this is now part of the library. Thanks for the changes!

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.

3 participants