-
Notifications
You must be signed in to change notification settings - Fork 31
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
Feature/c3s extract funs #110
Merged
Merged
Conversation
This file contains 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
- Removed duplicated code. - Made use of qualified names for cf_cariables. This hides in IndexConfig the expected order of input variable. - Simplified output type of funs. No need to create additional types.
It was messing with fully qualified named making python being lost, and it couldn't choose correctly between the package and the module.
For now, we store the yaml file internally. This is based on version 0.3.0 of the yaml file. We must follow updates from https://github.com/clix-meta/clix-meta
This qualifier metadata is used internally to figure out the necessary parameters to compute the index. We need it for C3S auto-generated functions.
This module will be used by C3S to extract icclim indices into individual functions.
Very impressive changes!
|
This comment has been minimized.
This comment has been minimized.
These changes are related to c3s integration.
`transfer_limit_Mbytes` was used to control dask chunking. This is however not really a good practice because all other dask configurations are still left outside icclim.
For project related reason this is not a 5.1.0 even if there are many changes since the initial 5.0 bump. This should not be an issue as icclim-v5 is not widely distributed yet.
- Overlapping years were not properly computed. - Chunking was done at dataset level but, it is only needed at data-array level.
This function list all available ecad indices.
bzah
force-pushed
the
feature/c3s_extract_funs
branch
from
January 27, 2022 16:13
8a56f4d
to
502362e
Compare
bzah
force-pushed
the
feature/c3s_extract_funs
branch
from
January 27, 2022 17:38
502362e
to
d26fa44
Compare
This could be improved by destructuring `user_index` parameter into multiple parameters on the generated function.
All changes are now under 5.0.0rc3, which is the targeted official release.
When the in_base and out_of_base fully overlap, it is unnecessary (and costly) to bootstrap percentiles.
bzah
force-pushed
the
feature/c3s_extract_funs
branch
from
January 28, 2022 17:30
3e48214
to
b53bebe
Compare
wip: - fill the todo - re-read
bzah
force-pushed
the
feature/c3s_extract_funs
branch
from
January 31, 2022 18:06
73888ba
to
d55441c
Compare
Also reformulate some sentences.
bzah
force-pushed
the
feature/c3s_extract_funs
branch
from
February 1, 2022 15:12
8f759d3
to
7523cf3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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 for the issue "C3S integration" (not logged in github)
Describe the changes you made
This PR covers multiple issues that c3s integration highlighted:
This makes pyyaml an required dependency of icclim.
Example of generated code with index TG (click me!)
Work in progress:
Also, for user who would try icclim first in c3s toolbox, it would exposes a similar API to them.
We should consider extracting user_index as well