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

More generic generic.stats - Allow 3H inputs #1498

Merged
merged 14 commits into from
Oct 16, 2023
Merged

Conversation

aulemahal
Copy link
Collaborator

@aulemahal aulemahal commented Oct 11, 2023

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes an issue raised in discussions with @vindelico
  • Tests for the changes have been added (for bug fixes / features)
    • (If applicable) Documentation has been added / updated (for bug fixes / features)
  • CHANGES.rst has been updated (with summary of main changes)
    • Link to issue (:issue:number) and pull request (:pull:number) has been added

What kind of change does this PR introduce?

  • Relax indicator generic.STATS so it accepts non-daily inputs. I saw no particular reason for it to be limit to daily values, except from the metadata.
  • Add variance to the supported op of to_agg_units.
  • Add a out_units to select_resample_op so that one can use it with an op to_agg_units doesn't know, and explicitly pass the expected units.

Does this PR introduce a breaking change?

Not a big one, but all YAML indicators previous constructed from select_resample_op should add out_units: null in their parameters section to remove the new argument from the signature. Altough its presence is not problematic in itself.

Other information:

@github-actions github-actions bot added the indicators Climate indices and indicators label Oct 11, 2023
@aulemahal aulemahal requested a review from huard October 11, 2023 21:22
@github-actions github-actions bot added the approved Approved for additional tests label Oct 11, 2023
@huard
Copy link
Collaborator

huard commented Oct 11, 2023

Maybe add a quick test ?

@aulemahal aulemahal changed the title More generic generic.stats More generic generic.stats - Allow 3H inputs Oct 11, 2023
@aulemahal
Copy link
Collaborator Author

Woups good call.

Because the initial issue of Marco was with 3H input, I tested with that. Turns out MissingBase wasn't able to comprehend frequencies containing multiples. I fixed that too.

CHANGES.rst Outdated Show resolved Hide resolved
xclim/core/units.py Show resolved Hide resolved
xclim/indicators/generic/_stats.py Show resolved Hide resolved
xclim/indices/generic.py Outdated Show resolved Hide resolved
@aulemahal aulemahal merged commit c11c06c into master Oct 16, 2023
12 checks passed
@aulemahal aulemahal deleted the genericer-generic branch October 16, 2023 21:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Approved for additional tests indicators Climate indices and indicators
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants