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

adjust repr tests to account for different platforms (#9127) #9128

Merged
merged 2 commits into from
Jun 16, 2024

Conversation

mgorny
Copy link
Contributor

@mgorny mgorny commented Jun 15, 2024

Adjust the expectations in repr tests to account for different object sizes and numpy type representations across platforms, particularly fixing the tests on 32-bit platforms.

Firstly, this involves getting the object type size from NumPy and using it to adjust the expectations in DataArray and Dataset tests. The tests were already using int64 type consistently, so only the sizes used for Python objects needed to be adjusted.

Secondly, this involves fixing test_array_repr_dtypes_unix. The test specifically focuses on testing a 32-bit, 64-bit and "native" data type, which affect both size and actual representation (NumPy skips the dtype attribute for the native data type). Get the expected size from NumPy for the native int type, and reuse repr() from NumPy for all array types.

Adjust the expectations in repr tests to account for different object
sizes and numpy type representations across platforms, particularly
fixing the tests on 32-bit platforms.

Firstly, this involves getting the object type size from NumPy and using
it to adjust the expectations in DataArray and Dataset tests.  The tests
were already using int64 type consistently, so only the sizes used
for Python objects needed to be adjusted.

Secondly, this involves fixing `test_array_repr_dtypes_unix`.  The test
specifically focuses on testing a 32-bit, 64-bit and "native" data type,
which affect both size and actual representation (NumPy skips the dtype
attribute for the native data type).  Get the expected size from NumPy
for the native int type, and reuse `repr()` from NumPy for all array
types.
Copy link
Collaborator

@max-sixty max-sixty left a comment

Choose a reason for hiding this comment

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

Very nice, thanks @mgorny

@max-sixty max-sixty added the plan to merge Final call for comments label Jun 15, 2024
Copy link
Collaborator

@headtr1ck headtr1ck 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.

Somewhat related is the script that creates the docstring examples which is also always giving the wrong numbers if run on Windows, this was annoying me already quite a few times... Would be a nice follow-up PR!

@max-sixty max-sixty merged commit 5ac8394 into pydata:main Jun 16, 2024
34 checks passed
@mgorny
Copy link
Contributor Author

mgorny commented Jun 16, 2024

Thanks!

@mgorny mgorny deleted the xarray-repr32 branch June 16, 2024 19:07
dcherian added a commit to dcherian/xarray that referenced this pull request Jun 21, 2024
* main:
  Split out distributed writes in zarr docs (pydata#9132)
  Update zendoo badge link (pydata#9133)
  Support duplicate dimensions in `.chunk` (pydata#9099)
  Bump the actions group with 2 updates (pydata#9130)
  adjust repr tests to account for different platforms (pydata#9127) (pydata#9128)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
plan to merge Final call for comments
Projects
None yet
Development

Successfully merging this pull request may close these issues.

test_repr_multiindex* and test_array_repr_dtypes_unix tests failing on 32-bit platforms
3 participants