-
-
Notifications
You must be signed in to change notification settings - Fork 365
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
Fix Ragged tests #1022
Fix Ragged tests #1022
Conversation
I think I've dealt with the final test failure. This PR needs approval again for the CI to run... |
@ianthomas23 It seems like we are endlessly chasing pandas expanding test suite for |
@philippjfr We could pin the version of pandas in There has been a discussion in numpy/scipy for such upper version pinning in the last year. |
Remaining test failures are
This seems to be a known problem with some combinations of The solution seems to be to pin the versions of one or the other. There might need to be some experimentation here to find a working combination. How do you want to proceed? Would you be happy merging this PR and then I can try to deal with it in another PR? This will be easier than adding it to this one as I will no longer be a first-time contributor so I should be able to use github actions without any assistance. |
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.
Looks good to me; I'll merge and other fixes can be made separately. Thanks, @ianthomas23, and sorry for the delay. Personally, I'd like to get all extension array code out of datashader and into e.g. SpatialPandas, but there are still a few important bits implemented only in Datashader. :-(
Same issues apply there, I don't think pinning pandas in setup.py is a great option, although it might be a good idea to do that in CI, because 99% of the time there's zero issue with the new pandas. It's just various new tests which don't apply to the ragged array and geometry arrays have to be skipped. |
@philippjfr I don't have any other cunning ideas for avoiding problems with future pandas releases. Maybe we just leave it as it is but try to check CI failures more frequently. Here at makepath we intend to use datashader a lot, so maybe we can help with future maintenance workload. I have a bunch of PR ideas lined up so that it is a bit more polished for our use, so I should start to get familiar with the code base. |
There are consistently 14 test failures in CI in the various
TestRagged
tests intest_datatypes.py
:The failures are due to a tightening-up of
numpy
andpandas
APIs and increased extension testing inpandas
.This PR removes the test failures; there are 4 types of fixes/changes:
pandas.util.testing
with equivalentnumpy
assertion.RaggedArray
, such as construction using nested sequences and indexing using ellipsis.As expert in
RaggedArray
andpandas
extension types could no doubt do a better job with item 4, but in the meantime I would quite like the CI to pass so that I can experiment with adding new functionality.