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

ENH: Fix Pandas 0.19.2 compatibility issues #1975

Closed
wants to merge 2 commits into from

Conversation

Peque
Copy link
Contributor

@Peque Peque commented Oct 4, 2017

Pandas 0.19 fixes some issues with Python 3.6 compatibility. It also adds new features and performance improvements. 0.19 is already 1 year old, so I guess it is already well-tested.

Note:

  • Had to add an ugly fix in tests/test_examples.py as I was not able to rebuild_example_data. Guess Benchmark downloading is broken #1965 is related.
  • Not sure the check_categorical=False is the best way to fix the tests. But I think a better fix would require deeper work (i.e.: remove the use of None/NaN in categorical data, as Pandas already warns about its future deprecation).

@Peque Peque force-pushed the pandas_upgrade branch 3 times, most recently from 5a0ddce to ce63a3d Compare October 5, 2017 18:14
@coveralls
Copy link

coveralls commented Oct 5, 2017

Coverage Status

Coverage remained the same at 87.202% when pulling ce63a3d on Peque:pandas_upgrade into 7cd6f6a on quantopian:master.

@freddiev4
Copy link
Contributor

Hi Miguel,

This looks like a great WIP, and seems like the fix for some issues you might be running into. Just wanted to message you in regards to some stuff here. So the Quantopian platform runs on pandas 0.18, and as a result, for us to make pandas 0.19 required would mean that many of our users would have go change their algorithms to be compatible with panda 0.19.

I think the most valuable thing here is to support pandas 0.19, and not make it required. That way the users on Quantopian are not affected by this change and so that you (along with other Zipline users) can continue to use Zipline.

Just figured I'd bring this up before you put in a bunch of other awesome work into this, and so there are less complications along the way.

@tibkiss
Copy link
Contributor

tibkiss commented Oct 8, 2017

@Peque : just curious: have you tried to bump it to 0.20?

@Peque Peque force-pushed the pandas_upgrade branch 18 times, most recently from d60b506 to cbe5ff4 Compare October 17, 2017 11:03
@coveralls
Copy link

coveralls commented Oct 17, 2017

Coverage Status

Coverage decreased (-0.4%) to 86.776% when pulling 5a6ddd8 on Peque:pandas_upgrade into 45257eb on quantopian:master.

@Peque Peque changed the title ENH: Bump required Pandas version to 0.19.2 ENH: Fix Pandas 0.19.2 compatibility issues Oct 17, 2017
@coveralls
Copy link

coveralls commented Oct 17, 2017

Coverage Status

Coverage remained the same at 87.202% when pulling 5a6ddd8 on Peque:pandas_upgrade into 45257eb on quantopian:master.

@coveralls
Copy link

coveralls commented Oct 17, 2017

Coverage Status

Coverage increased (+0.01%) to 87.215% when pulling a636660 on Peque:pandas_upgrade into 45257eb on quantopian:master.

Peque added 2 commits October 17, 2017 15:28
For those cases in which other type of warnings (i.e.: RuntimeWarning,
that could be thrown by NumPy) should not affect the tests results.
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 86.789% when pulling 1002c58 on Peque:pandas_upgrade into 45257eb on quantopian:master.

@coveralls
Copy link

coveralls commented Oct 17, 2017

Coverage Status

Coverage increased (+0.01%) to 87.215% when pulling 1002c58 on Peque:pandas_upgrade into 45257eb on quantopian:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.4%) to 86.789% when pulling 1002c58 on Peque:pandas_upgrade into 45257eb on quantopian:master.

@Peque
Copy link
Contributor Author

Peque commented Oct 17, 2017

@freddiev4 Does it look better now?

@Peque
Copy link
Contributor Author

Peque commented Oct 17, 2017

@tibkiss Yeah, that resulted in new errors popping out, so I guess it is better to go step by step. 😊

@Peque
Copy link
Contributor Author

Peque commented Oct 31, 2017

@freddiev4 Friendly ping. 😊

@freddiev4
Copy link
Contributor

freddiev4 commented Jul 16, 2018

Considering we merged #2194 should we close this @richafrank? In favor of #2243

@richafrank
Copy link
Member

Agreed

@richafrank richafrank closed this Jul 17, 2018
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