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

Lazy weighted RMS calculation #5017

Merged
merged 5 commits into from
Feb 27, 2023
Merged

Conversation

rcomer
Copy link
Member

@rcomer rcomer commented Oct 7, 2022

🚀 Pull Request

Description

Lazy aggregation with weights was not previously implemented for the RMS aggregator because of a bug in the weighted masked case reported at dask/dask#3846 (see removed code comments). This bug was fixed at dask/dask#4236, and the fix included in version 1.1.0. Iris's minimum supported version of Dask is now 2.26.


Consult Iris pull request check list

@bjlittle
Copy link
Member

@rcomer do you have time to rebase this PR? 😄

Note that the referenced dask issue was fixed by dask#4236 which was included
in v1.1.0.
Copy link
Contributor

@ESadek-MO ESadek-MO left a comment

Choose a reason for hiding this comment

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

Hey @rcomer! Looks great, I think the PR number might be wrong in the What's New, but otherwise looks good to go!

docs/src/whatsnew/latest.rst Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 27, 2023

Codecov Report

❗ No coverage uploaded for pull request base (main@d63e629). Click here to learn what that means.
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #5017   +/-   ##
=======================================
  Coverage        ?   89.24%           
=======================================
  Files           ?       88           
  Lines           ?    22195           
  Branches        ?     4857           
=======================================
  Hits            ?    19809           
  Misses          ?     1641           
  Partials        ?      745           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@ESadek-MO ESadek-MO merged commit 21b8a2c into SciTools:main Feb 27, 2023
@rcomer rcomer deleted the rms-lazy-weighted branch February 27, 2023 13:04
tkknight added a commit to tkknight/iris that referenced this pull request Apr 22, 2023
* upstream/main: (23 commits)
  Lockfiles and pydata-sphinx-theme fix (SciTools#5188)
  Allow smarter weights (cubes, coordinates, cell measures, or ancillary variables) for aggregation (SciTools#5084)
  removed cell measure mask check and error (SciTools#5181)
  Updated environment lockfiles (SciTools#5177)
  Lazy weighted RMS calculation (SciTools#5017)
  Add coverage badge to README.md (SciTools#5176)
  Add coverage testing (SciTools#4765)
  Whats new updates for v3.4.1 .
  NetCDF thread safety take two (SciTools#5095)
  Updated environment lockfiles (SciTools#5163)
  Plugin support (SciTools#5144)
  Expand scope of common contributor links (SciTools#5159)
  Replace apparently retired UDUNITS documentation link. (SciTools#5153)
  [pre-commit.ci] pre-commit autoupdate (SciTools#5150)
  Fixing typo's in Gitwash. (SciTools#5145)
  add readme #showyourstripes (SciTools#5141)
  [pre-commit.ci] pre-commit autoupdate (SciTools#5143)
  Iris ❤ Xarray docs page. (SciTools#5025)
  [pre-commit.ci] pre-commit autoupdate (SciTools#5136)
  Updated citation (SciTools#5116)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants