Skip to content

Expression not assigned #10507

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

Merged
merged 3 commits into from
Jul 9, 2025
Merged

Expression not assigned #10507

merged 3 commits into from
Jul 9, 2025

Conversation

DimitriPapadopoulos
Copy link
Contributor

  • Closes #xxxx
  • Tests added
  • User visible changes (including notable bug fixes) are documented in whats-new.rst
  • New functions/methods are listed in api.rst

@@ -1129,7 +1129,8 @@ def test_unify_chunks(map_ds):
def test_unify_chunks_shallow_copy(obj, transform):
obj = transform(obj)
unified = obj.unify_chunks()
assert_identical(obj, unified) and obj is not obj.unify_chunks()
assert_identical(obj, unified)
# assert obj is not obj.unify_chunks()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
# assert obj is not obj.unify_chunks()
assert obj is not obj.unify_chunks()

I think this may be right

Copy link
Contributor Author

@DimitriPapadopoulos DimitriPapadopoulos Jul 7, 2025

Choose a reason for hiding this comment

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

The test doesn't work, that's why it has been commented out. I should probably improve the comment in commit 5f93e2d.

Besides I think it should have been:

Suggested change
# assert obj is not obj.unify_chunks()
assert obj is not unified

Copy link
Contributor Author

@DimitriPapadopoulos DimitriPapadopoulos Jul 8, 2025

Choose a reason for hiding this comment

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

Uncommenting the test demonstrates the CI issue:

=========================== short test summary info ============================
FAILED xarray/tests/test_dask.py::test_unify_chunks_shallow_copy[<lambda>0-obj0] - assert <xarray.Dataset> Size: 9kB\nDimensions:  (x: 10, y: 20, z: 4)\nCoordinates:\n  * x        (x) int64 80B 0 1 2 3 4 5 6 7 8...2B 1 1 1 1\n    e        (x, y) int64 2kB 100 101 102 103 104 105 ... 124 125 126 127 128\nAttributes:\n    test:     test is not <xarray.Dataset> Size: 9kB\nDimensions:  (x: 10, y: 20, z: 4)\nCoordinates:\n  * x        (x) int64 80B 0 1 2 3 4 5 6 7 8...2B 1 1 1 1\n    e        (x, y) int64 2kB 100 101 102 103 104 105 ... 124 125 126 127 128\nAttributes:\n    test:     test
FAILED xarray/tests/test_dask.py::test_unify_chunks_shallow_copy[<lambda>0-obj1] - AssertionError: assert <xarray.DataArray 'a' (x: 10, y: 20)> Size: 2kB\narray([[1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1....2 14 16 18\n    cxy      (x, y) int64 2kB 0 0 0 0 0 0 0 ... 1026 1035 1044 1053 1062 1071\nAttributes:\n    test:     test is not <xarray.DataArray 'a' (x: 10, y: 20)> Size: 2kB\narray([[1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1., 1....2 14 16 18\n    cxy      (x, y) int64 2kB 0 0 0 0 0 0 0 ... 1026 1035 1044 1053 1062 1071\nAttributes:\n    test:     test
= 2 failed, 19121 passed, 1041 skipped, 195 xfailed, 33 xpassed, 2864 warnings in 348.89s (0:05:48) =
Error: Process completed with exit code 1.

See for example https://github.com/pydata/xarray/actions/runs/16135835929/job/45531837533?pr=10507.

@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review July 7, 2025 17:38
I guess the `and` was meant to disable the assertion.
Commenting it out with `#` instead.
@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as draft July 8, 2025 06:36
@DimitriPapadopoulos DimitriPapadopoulos marked this pull request as ready for review July 8, 2025 07:06
@dcherian dcherian merged commit c43a374 into pydata:main Jul 9, 2025
54 of 71 checks passed
@DimitriPapadopoulos DimitriPapadopoulos deleted the PYL-W0106 branch July 9, 2025 15:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants