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

pysteps.utils with xarray #223

Merged
merged 35 commits into from
Nov 26, 2021
Merged

pysteps.utils with xarray #223

merged 35 commits into from
Nov 26, 2021

Conversation

dnerini
Copy link
Member

@dnerini dnerini commented Jul 28, 2021

Include new xarray-based data model into the pysteps.utils module (See #12).

utils.conversion

  • remove metadata arguments (since all attributes are now part of the input xr.DataArray object)
  • make all methods available as xarray accessors for DataArrays (e.g. can use both the data_array.pysteps.to_rainrate() or pysteps.utils.to_rainrate(data_array) syntax)
  • to_reflectivity() has the option to convert to dB [dBZ] or keep linear scale [Z]
  • add type hints for inputs and outputs
  • reduce code duplication
  • adapt tests

utils.dimension (DEPRECATED)

  • deprecate aggregate_fields_time in favor of xarray's coarsen method.
  • deprecate aggregate_fields_space in favor of xarray's coarsen method.
  • deprecate aggregate_fields in favor of xarray's coarsen method.
  • deprecate clip_domain in favor of xarray's sel method.
  • deprecate square_domain since it is obsolete.

utils.images

  • morph_opening takes xr.DataArray as input, and it also handles cases with time dimension "t"
  • argument n of morph_opening is renamed kernel and can either be the size of the kernel or the kernel itself.
  • update tests

utils.interpolate

  • all interpolation methods take and return xarray's object (both Dataset and DataArray)
  • migrate deprecated scipy's Rbf to new RBFInterpolator (scipy>=1.7)
  • update tests

utils.transformation

  • remove metadata arguments (since all attributes are now part of the input xr.DataArray object)
  • make all methods available as xarray accessors for DataArrays (e.g. can use both the data_array.pysteps.sqrt_transform() or pysteps.utils.sqrt_transform(data_array) syntax)
  • add general back_transform method to remove any existing data transformation.
  • boxcox_transform and db_transform use a simpler offset to avoid computing log(0), instead of having to define the threshold and zerovalue parameters as in V1 (closes dB_transform ignores existing metadata #198)
  • refactor nq_transform: in particular, the inverse transformation needs an empirical target distribution (the template argument).
  • add type hints for inputs and outputs
  • reduce code duplication
  • rewrite tests

@dnerini dnerini self-assigned this Jul 28, 2021
@dnerini dnerini mentioned this pull request Aug 11, 2021
Also migrate to RBFInterpolator (scipy>=1.7)
@dnerini dnerini linked an issue Oct 12, 2021 that may be closed by this pull request
@dnerini dnerini marked this pull request as ready for review October 12, 2021 14:03
@dnerini
Copy link
Member Author

dnerini commented Oct 12, 2021

OK let's say this is ready for review!

  • still failing tests (need to adapt all methods that use the utils module)
  • didn't touch at the utils.spectral module for now, do we want to adopt xarray for complex data too? this would be similar to what is done in xrft. Proposition: should we adopt xrft ? @pulkkins

I know it is a lot of changes, I don't expect you to go through all of them. Rather, please try to see whether you agree with the general changes in the interfaces listed in the summary at the top.

Thanks!

@RubenImhoff
Copy link
Contributor

This is an amazing amount of work, @dnerini! I'll try to have a look at it soon.

Copy link
Contributor

@RubenImhoff RubenImhoff left a comment

Choose a reason for hiding this comment

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

Amazing work, @dnerini! Because you've done so much work, I can't promise that I've spotted all possible mistakes in the code, so let's also wait for another review by one of the others. ;)

@@ -1,4 +1,5 @@
# coding: utf-8
# coding: utf-8
Copy link
Contributor

Choose a reason for hiding this comment

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

Very minor: I believe we have:
# -*- coding: utf-8 -*-
in most of the scripts

examples/anvil_nowcast.py Show resolved Hide resolved
pysteps/io/exporters.py Outdated Show resolved Hide resolved
pysteps/io/nowcast_importers.py Show resolved Hide resolved
pysteps/tests/test_io_mrms_grib.py Show resolved Hide resolved
pysteps/utils/conversion.py Show resolved Hide resolved
pysteps/utils/interface.py Outdated Show resolved Hide resolved
dnerini and others added 2 commits November 26, 2021 12:04
Co-authored-by: Ruben Imhoff <31476760+RubenImhoff@users.noreply.github.com>
@dnerini dnerini merged commit bee4b2f into pysteps-v2 Nov 26, 2021
@dnerini dnerini deleted the utils-xarray branch November 26, 2021 11:07
@dbkhout dbkhout mentioned this pull request Dec 22, 2022
Sign up for free to 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.

dB_transform ignores existing metadata
2 participants