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

Per-variable specification of boolean parameters in open_dataset #9218

Merged
merged 17 commits into from
Jul 16, 2024

Conversation

Ostheer
Copy link
Contributor

@Ostheer Ostheer commented Jul 8, 2024

Added feature to choose whether to apply mask_and_scale for each DataArray in a Dataset individually.

The mask_and_scale parameter can now not only be bool, but also a mapping from variable name (str) to bool.

- [?] User visible changes (including notable bug fixes) are documented in whats-new.rst

  • New functions/methods are listed in api.rst

I wasn't sure if I was supposed to edit whats-new.rst, so I didn't touch it.

@Ostheer
Copy link
Contributor Author

Ostheer commented Jul 12, 2024

Can someone tell me if and how I should edit the what's new file?

I could add something like

- Allow per-variable specification of `mask_and_scale` in `open_dataset`  (:pull:`9218`).
  By `Mathijs Verhaegh <https://github.com/Ostheer>`_.

But I'm not sure where, as I don't know into which upcoming release this commit would be included.

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

This looks good!

Great re whatsnew suggestion, within New Features works!

xarray/backends/api.py Outdated Show resolved Hide resolved
xarray/conventions.py Outdated Show resolved Hide resolved
@headtr1ck
Copy link
Collaborator

Could also easily extend to other arguments, like decode_times. What do you think?

Ostheer and others added 4 commits July 13, 2024 00:39
Co-authored-by: Michael Niklas  <mick.niklas@gmail.com>
Otherwise you lose all typing when you use that because it returns Any.
@Ostheer
Copy link
Contributor Author

Ostheer commented Jul 12, 2024

Could also easily extend to other arguments, like decode_times. What do you think?

Added for 4 more params (46de50f)

Feature now affects:

mask_and_scale, decode_times, decode_timedelta, use_cftime, concat_characters

@Ostheer Ostheer changed the title Granular mask and scale Per-variable specification of boolean parameters in open_dataset Jul 12, 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.

Add an entry in what's new and this is good! Thanks

xarray/conventions.py Outdated Show resolved Hide resolved
@Ostheer
Copy link
Contributor Author

Ostheer commented Jul 13, 2024

I resolved some minor Mypy issues stemming from the annotations on _item_or_default, but the Mypy and Mypy 3.9 tasks are still failing.

I can't find how the errors in the logs relate to any of the code changes. Any ideas?

@headtr1ck
Copy link
Collaborator

I resolved some minor Mypy issues stemming from the annotations on _item_or_default, but the Mypy and Mypy 3.9 tasks are still failing.

I can't find how the errors in the logs relate to any of the code changes. Any ideas?

These checks are failing on main as well. There are already some PRs open to fix these.

I would say this looks good.

@Ostheer
Copy link
Contributor Author

Ostheer commented Jul 13, 2024

I resolved some minor Mypy issues stemming from the annotations on _item_or_default, but the Mypy and Mypy 3.9 tasks are still failing.
I can't find how the errors in the logs relate to any of the code changes. Any ideas?

These checks are failing on main as well. There are already some PRs open to fix these.

I would say this looks good.

I see. Can we then somehow merge this despite the failing checks? I don't think Github lets me.

@headtr1ck
Copy link
Collaborator

Yes, no worries.
Let's keep it a day or two for other people to check and then merge. Thanks!

xarray/backends/api.py Outdated Show resolved Hide resolved
@max-sixty
Copy link
Collaborator

Thanks @Ostheer !

@max-sixty max-sixty merged commit 7477fd1 into pydata:main Jul 16, 2024
24 of 28 checks passed
Copy link

welcome bot commented Jul 16, 2024

Congratulations on completing your first pull request! Welcome to Xarray! We are proud of you, and hope to see you again! celebration gif

dcherian added a commit to dcherian/xarray that referenced this pull request Jul 17, 2024
* main:
  Enable pandas type checking (pydata#9213)
  Per-variable specification of boolean parameters in open_dataset (pydata#9218)
  test push
  Added a space to the documentation (pydata#9247)
  Fix typing for test_plot.py (pydata#9234)
  Allow mypy to run in vscode (pydata#9239)
  Revert "Test main push"
  Test main push
  Revert "Update _typing.py"
  Update _typing.py
  Add a `.drop_attrs` method (pydata#8258)
dcherian added a commit that referenced this pull request Jul 22, 2024
* main:
  add backend intro and how-to diagram (#9175)
  Fix copybutton for multi line examples in double digit ipython cells (#9264)
  Update signature for _arrayfunction.__array__ (#9237)
  Add encode_cf_datetime benchmark (#9262)
  groupby, resample: Deprecate some positional args (#9236)
  Delete ``base`` and ``loffset`` parameters to resample (#9233)
  Update dropna docstring (#9257)
  Grouper, Resampler as public api (#8840)
  Fix mypy on main (#9252)
  fix fallback isdtype method (#9250)
  Enable pandas type checking (#9213)
  Per-variable specification of boolean parameters in open_dataset (#9218)
  test push
  Added a space to the documentation (#9247)
  Fix typing for test_plot.py (#9234)
dcherian added a commit that referenced this pull request Jul 24, 2024
* main: (54 commits)
  Adding `open_datatree` backend-specific keyword arguments (#9199)
  [pre-commit.ci] pre-commit autoupdate (#9202)
  Restore ability to specify _FillValue as Python native integers (#9258)
  add backend intro and how-to diagram (#9175)
  Fix copybutton for multi line examples in double digit ipython cells (#9264)
  Update signature for _arrayfunction.__array__ (#9237)
  Add encode_cf_datetime benchmark (#9262)
  groupby, resample: Deprecate some positional args (#9236)
  Delete ``base`` and ``loffset`` parameters to resample (#9233)
  Update dropna docstring (#9257)
  Grouper, Resampler as public api (#8840)
  Fix mypy on main (#9252)
  fix fallback isdtype method (#9250)
  Enable pandas type checking (#9213)
  Per-variable specification of boolean parameters in open_dataset (#9218)
  test push
  Added a space to the documentation (#9247)
  Fix typing for test_plot.py (#9234)
  Allow mypy to run in vscode (#9239)
  Revert "Test main push"
  ...
dcherian added a commit to JoelJaeschke/xarray that referenced this pull request Jul 25, 2024
…monotonic-variable

* main: (995 commits)
  Adding `open_datatree` backend-specific keyword arguments (pydata#9199)
  [pre-commit.ci] pre-commit autoupdate (pydata#9202)
  Restore ability to specify _FillValue as Python native integers (pydata#9258)
  add backend intro and how-to diagram (pydata#9175)
  Fix copybutton for multi line examples in double digit ipython cells (pydata#9264)
  Update signature for _arrayfunction.__array__ (pydata#9237)
  Add encode_cf_datetime benchmark (pydata#9262)
  groupby, resample: Deprecate some positional args (pydata#9236)
  Delete ``base`` and ``loffset`` parameters to resample (pydata#9233)
  Update dropna docstring (pydata#9257)
  Grouper, Resampler as public api (pydata#8840)
  Fix mypy on main (pydata#9252)
  fix fallback isdtype method (pydata#9250)
  Enable pandas type checking (pydata#9213)
  Per-variable specification of boolean parameters in open_dataset (pydata#9218)
  test push
  Added a space to the documentation (pydata#9247)
  Fix typing for test_plot.py (pydata#9234)
  Allow mypy to run in vscode (pydata#9239)
  Revert "Test main push"
  ...
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 topic-backends
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants