Skip to content

Conversation

jsignell
Copy link
Contributor

@jsignell jsignell commented Aug 1, 2025

So far this PR just adds some tests demonstrating how Using certain join values ("exact" and "override") on merge can alter the original objects.

I discovered this in the context of #10062 (comment) but it's not really related to changes in that PR.

  • 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

@jsignell
Copy link
Contributor Author

jsignell commented Aug 1, 2025

@dcherian I am taking a look and will report back findings.

@dcherian
Copy link
Contributor

dcherian commented Aug 1, 2025

Thanks! we have had this kind of issue before: #10330 Maybe a helper function is in order

@jsignell
Copy link
Contributor Author

jsignell commented Aug 1, 2025

#7736 mentions regression so we will want to make sure that we don't solve this by just copying aggressively.

@jsignell
Copy link
Contributor Author

jsignell commented Aug 1, 2025

Thanks for the pointer to #10330! I just took that logic and moved it outside the if block so that everybody has to go through it. I think the remaining tests are the same that are failing on main.

@jsignell
Copy link
Contributor Author

jsignell commented Aug 1, 2025

See #10599 for fixes to doctests and mypy.

@jsignell
Copy link
Contributor Author

jsignell commented Aug 1, 2025

It would probably be a good idea to run benchmarks on this PR.

@jsignell jsignell changed the title ds.merge can alter original object depending on join value Fix ds.merge to prevent altering original object depending on join value Aug 4, 2025
@jsignell jsignell added the run-benchmark Run the ASV benchmark workflow label Aug 7, 2025
Copy link
Contributor

@dcherian dcherian left a comment

Choose a reason for hiding this comment

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

LGTM

@dcherian dcherian merged commit 44d5fd6 into pydata:main Aug 14, 2025
36 of 37 checks passed
@jsignell jsignell deleted the join-copy branch August 19, 2025 19:37
dcherian added a commit to dhruvak001/xarray that referenced this pull request Aug 24, 2025
* main: (46 commits)
  use the new syntax of ignoring bots (pydata#10668)
  modification methods on `Coordinates` (pydata#10318)
  Silence warnings from test_tutorial.py (pydata#10661)
  test: update write_empty test for zarr 3.1.2 (pydata#10665)
  Bump actions/checkout from 4 to 5 in the actions group (pydata#10652)
  Add load_datatree function (pydata#10649)
  Support compute=False from DataTree.to_netcdf (pydata#10625)
  Fix typos (pydata#10655)
  In case of misconfiguration of dataset.encoding `unlimited_dims` warn instead of raise (pydata#10648)
  fix ``auto_complex`` for ``open_datatree`` (pydata#10632)
  Fix bug indexing with boolean scalars (pydata#10635)
  Improve DataTree typing (pydata#10644)
  Update Cartopy and Iris references (pydata#10645)
  Empty release notes (pydata#10642)
  release notes for v2025.08.0 (pydata#10641)
  Fix `ds.merge` to prevent altering original object depending on join value (pydata#10596)
  Add asynchronous load method (pydata#10327)
  Add DataTree.prune() method              … (pydata#10598)
  Avoid refining parent dimensions in NetCDF files (pydata#10623)
  clarify lazy behaviour and eager loading chunks=None in open_*-functions (pydata#10627)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-benchmark Run the ASV benchmark workflow
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants