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 numpy2 compatibility #183

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

SimonSegerblomRex
Copy link

Copy link
Owner

@pearu pearu left a comment

Choose a reason for hiding this comment

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

I have one change request, otherwise LGTM!

Thanks, @SimonSegerblomRex

pyproject.toml Show resolved Hide resolved
@pearu pearu requested a review from djhoese June 28, 2024 14:32
numpy2 doesn't support 3.8.
Not sure if something more needs to be
updated here..?
@SimonSegerblomRex
Copy link
Author

SimonSegerblomRex commented Jun 28, 2024

I have one change request, otherwise LGTM!

Thanks, @SimonSegerblomRex

Thanks! I updated ci.yaml as well now to not include 3.8.

@pearu
Copy link
Owner

pearu commented Jun 28, 2024

Re:

>           total_size -= sdiff
E           OverflowError: Python integer -7812 out of bounds for uint32

I think the resolution here is to make sure that total_size (and perhaps sdiff) is Python int, not numpy uint32.

@SimonSegerblomRex
Copy link
Author

Re:

Thanks! Now the tests pass locally on my machine at least, but it still feels a bit shaky.

djhoese
djhoese previously approved these changes Jun 29, 2024
Copy link
Collaborator

@djhoese djhoese left a 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 from what I understand. Thanks for doing this!

libtiff/utils.py Outdated Show resolved Hide resolved
@SimonSegerblomRex
Copy link
Author

Perhaps some of the MacOS ARM64 failures could be fixed by #179..?

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