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 numpy scalar hash for bigendian #285

Merged
merged 7 commits into from
Jan 17, 2025

Conversation

matthiasdiener
Copy link
Contributor

@matthiasdiener matthiasdiener commented Jan 16, 2025

Fixes #262. Followup of #259, #260, #261.

Reverts #260.
Closes #261.

TODO:

  • add a test with mock

Please squash

@matthiasdiener matthiasdiener changed the title Fix numpy repr fix numpy hash for bigendian Jan 16, 2025
@matthiasdiener matthiasdiener force-pushed the fix-numpy-repr branch 2 times, most recently from 99dc1b9 to 4ab016c Compare January 16, 2025 23:38
@matthiasdiener
Copy link
Contributor Author

This is ready for a first look @inducer @mgorny

@matthiasdiener matthiasdiener marked this pull request as ready for review January 16, 2025 23:43
@matthiasdiener matthiasdiener changed the title fix numpy hash for bigendian fix numpy scalar hash for bigendian Jan 16, 2025
Copy link
Contributor

@mgorny mgorny left a comment

Choose a reason for hiding this comment

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

Tests pass for me on PPC64.

Co-authored-by: Michał Górny <[email protected]>
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

Two minor nits, otherwise good to go.

@@ -68,6 +68,9 @@ class RecommendedHashNotFoundWarning(UserWarning):
_HAS_ATTRS = True


IS_BIGENDIAN = sys.byteorder == "big"
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
IS_BIGENDIAN = sys.byteorder == "big"
_IS_BIGENDIAN = sys.byteorder == "big"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 49820c7

@@ -68,6 +68,9 @@ class RecommendedHashNotFoundWarning(UserWarning):
_HAS_ATTRS = True


IS_BIGENDIAN = sys.byteorder == "big"
Copy link
Owner

Choose a reason for hiding this comment

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

Add a comment that this is here so it can be changed by mock testing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in 0194fc6

@inducer inducer merged commit c0364fd into inducer:main Jan 17, 2025
17 checks passed
@matthiasdiener matthiasdiener deleted the fix-numpy-repr branch January 17, 2025 20:00
@mgorny
Copy link
Contributor

mgorny commented Jan 17, 2025

Thanks a lot, to you both!

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.

pytools/test/test_persistent_dict.py::test_{attrs,dataclass}_hashing failures on big-endian
3 participants