Conversation
Collaborator
Author
|
@bzah With this PR, I modified the way one can specifiy which input frequencies are accepted. See the The good part is that one can now inherit from an indicator created from |
Pull Request Test Coverage Report for Build 1481920829
💛 - Coveralls |
Contributor
|
Thank you for your concern (and for the feature!) @aulemahal, it won't break my code I'm using atmos daily indicators directly and I have simply disabled the daily check for now on icc v5. |
Co-authored-by: David Huard <huard.david@ouranos.ca>
raquelalegre
added a commit
to UCL/xclim
that referenced
this pull request
Nov 30, 2021
…s/heat_index * 'indices/heat_index' of github.com:UCL/xclim: (42 commits) Added french metadata for heat index. Apply changes to fraction indicator [sdba] Re-write of Extreme's adjustment (Ouranosinc#914) update history Update history [Ouranosinc#931] Fix indicator for Rxxp index Add missing type for `threshold_count` Add dev notes - add indexer example - other small changes run all available tests in one call for slow builds update makefile target black, only install coveralls in tox if coveralls is needed Indexing within indicators (Ouranosinc#934) Update history finish needs doctests Remove unnecessary keep_attrs Adapt to CF conventions 1.9 - remove gregorian calendar (Ouranosinc#935) Bump version: 0.31.2-beta → 0.31.3-beta update HISTORY.rst suggested corrections Apply suggestions from code review Resampling indicator (Ouranosinc#927) ...
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Checklist:
number) and pull request (:pull:number) has been added.bumpversion patchhas been called on this branch.zenodo.jsonWhat kind of change does this PR introduce?
ResamplingIndicatorsubclass. I did this by creating private pre- and post-processing methods that are called in__call__. This makes it easy to "inject" functionalities in the indicator's processing. The "resampling-related functionalities" are : checking for the allowed "freq" arg and handling of missing values.Does this PR introduce a breaking change?
I don't think so.
Other information:
I think that with this change it becomes a bit clearer what are the uses cases for conventional subclassing of the
Indicatorclass:context,src_freq,realm,title,abstract,keywords,references,notesandallowed_periods.In both cases, the goal is to create a new base class and then to create multiple indicators.
I'll put this in a "developer's note" section of the doc in a further PR.
This is also a first step towards a way to inject the
indexerkwargs as planned in #899.