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

Fix annotate for chunked operations like in insulation #422

Merged
merged 7 commits into from
Jun 17, 2024
Merged

Conversation

Phlya
Copy link
Member

@Phlya Phlya commented May 31, 2024

Simple solution with .loc instead of .iloc - probaly a bit slower, perhaps some arithmetic can fix the .iloc to behave correctly

@Phlya Phlya requested a review from nvictus May 31, 2024 15:11
@Phlya
Copy link
Member Author

Phlya commented Jun 13, 2024

I had to add the recently removed export of _IndexingMixin, since cooltools uses it...

@Phlya
Copy link
Member Author

Phlya commented Jun 13, 2024

@nvictus would you be able to review? We should release a fix asap

@nvictus
Copy link
Member

nvictus commented Jun 14, 2024

This needs a test case that triggers the error without the fix.

@nvictus
Copy link
Member

nvictus commented Jun 14, 2024

I had to add the recently removed export of _IndexingMixin, since cooltools uses it...

We can restore it in a patch, but cooltools needs to vendor it in, because it is not part of cooler's public API and I'll remove it again in the next minor release.

src/cooler/api.py Outdated Show resolved Hide resolved
@Phlya
Copy link
Member Author

Phlya commented Jun 17, 2024

Thank you @nvictus!
For now I am keeping the _IndexingMixin, since without it cooltools just breaks completely... We can migrate it in the next cooltools release and then remove from cooler.
Added a test, it fails in the master branch but works in the version here in my local tests.

Of course numpy 2.0 breaks stuff, so the tests here fail...

@nvictus
Copy link
Member

nvictus commented Jun 17, 2024

Great opportunity to throw in the numpy major version pin.

@Phlya
Copy link
Member Author

Phlya commented Jun 17, 2024

Looks like disallowing numpy 2.0 makes the tests pass! Shall we merge and make a bugfix release?

@nvictus nvictus merged commit 32eb132 into master Jun 17, 2024
8 checks passed
@nvictus nvictus deleted the fix-annotate branch June 17, 2024 19:58
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.

2 participants