-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Update concat for multi-variable indexes. #10371
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
base: main
Are you sure you want to change the base?
Conversation
When concatenating along a dimension associated with a multi-variable index, avoid merging the other dimensions. Instead we rely on a strict equality check along those dimensions (7551a7a)
988503e
to
a996655
Compare
Co-authored-by: Benoit Bovy <[email protected]>
* main: Fix performance regression in interp from pydata#9881 (pydata#10370) html repr: improve style for dropdown sections (pydata#10354) Grouper tweaks. (pydata#10362) Docs: Add links to getting help mermaid diagram (pydata#10324) Enforce ruff/flynt rules (FLY) (pydata#10375) Add missing AbstractWritableDataStore base methods and arguments (pydata#10343) Improve html repr in dark mode (Jupyterlab + Xarray docs) (pydata#10353) Pin Mypy to 1.15 (pydata#10378) use numpy dtype exposed by zarr array instead of metadata.data_type (pydata#10348) Fix doc typo for caption "Interoperability" (pydata#10374) Implement cftime vectorization as discussed in PR pydata#8322 (pydata#8324) Enforce ruff/flake8-pyi rules (PYI) (pydata#10359) Apply assorted ruff/Pylint rules (PL) / Enforce PLE rules (pydata#10366) (fix): pandas extension array repr for int64[pyarrow] (pydata#10317) Enforce ruff/flake8-implicit-str-concat rules (ISC) (pydata#10368) Enforce ruff/refurb rules (FURB) (pydata#10367) Ignore ruff/Pyflakes rule F401 more precisely (pydata#10369) Apply assorted ruff/flake8-simplify rules (SIM) (pydata#10364) Apply assorted ruff/flake8-pytest-style rules (PT) (pydata#10363) Fix "a array" misspelling (pydata#10365)
xarray/tests/indexes.py
Outdated
@@ -0,0 +1,79 @@ | |||
from collections.abc import Hashable, Iterable, Sequence |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
refactored these out from test_dataset.py
so they can be reused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
xarray/tests/indexes.py
Outdated
y=PandasIndex.from_variables({"y": variables["y"]}, options=options), | ||
) | ||
|
||
def create_variables(self, variables): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added this
return x_eq and y_eq | ||
|
||
@classmethod | ||
def concat( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added this
# skip merging these variables. | ||
# if concatenating over a dimension 'x' that is associated with an index over 2 variables, | ||
# 'x' and 'y', then we assert join="equals" on `y` and don't need to merge it. | ||
# that assertion happens in the align step prior to this function being called |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was wondering how this would behave for the corner case where the "y" coordinate (and/or the "x" coordinate) does not have any index in one or more of the objects to concatenate. We could leave it for now, though. It is quite unlikely that it will occur in practice I'd say.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what you mean, we defer to the Index, so it can do what's sensible?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @dcherian!
Co-authored-by: Benoit Bovy <[email protected]>
When concatenating along a dimension associated with a multi-variable index, avoid merging the other dimensions. Instead we rely on a strict equality check along those dimensions (7551a7a)
Still needs tests, but seems to work on dcherian/rasterix#18