Skip to content

add a combine_attrs option to open_mfdataset - #4971

Merged
keewis merged 7 commits into
pydata:masterfrom
keewis:open_mfdataset-combine_attrs
Apr 3, 2021
Merged

add a combine_attrs option to open_mfdataset#4971
keewis merged 7 commits into
pydata:masterfrom
keewis:open_mfdataset-combine_attrs

Conversation

@keewis

@keewiskeewis commented Feb 27, 2021

Copy link
Copy Markdown
Collaborator

In order to fix the failing tests in #4902 we need to expose combine_attrs to be able to properly construct the expected result (to be passed through to the combine function).

This overlaps with the fallback of the attrs_file code, which I removed for now. Maybe combine_attrs="override" would be better?

  • Tests added
  • Passes pre-commit run --all-files
  • User visible changes (including notable bug fixes) are documented in whats-new.rst

@mathause

Copy link
Copy Markdown
Collaborator

The motivation for attrs_file is given in #2382. Maybe require combine_attrs == "override" when attrs_file is not None?

The current behavior is combine_attrs="override", right? So when choosing something else this would have to be deprecated

@keewis

Copy link
Copy Markdown
CollaboratorAuthor

for the main object, yes, but for the variables of that object "drop" is currently always used. That means that if we try to sync both we introduce a breaking change.

We might eventually want to switch both to "drop_conflicts" so we would need to go through a deprecation cycle anyways.

@keewis

Copy link
Copy Markdown
CollaboratorAuthor

I decided to change the default from "drop" to "override", so the only thing that changes is that the attrs for variables will not be dropped but kept according to combine_attrs.

I guess that means that unless the CI fails this should be ready for review / merging?

@keewis

Copy link
Copy Markdown
CollaboratorAuthor

(the RTD build fails because docs.scipy.org is down)

@keewis

Copy link
Copy Markdown
CollaboratorAuthor

should we merge this? It is blocking #4902, which in turn is blocking #5041.

Comment threadxarray/tests/test_backends.py
@keewis
keewis merged commit 3cbd21a into pydata:masterApr 3, 2021
@keewis
keewis deleted the open_mfdataset-combine_attrs branch April 3, 2021 15:43
Sign up for freeto 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.

3 participants

@keewis@mathause@dcherian