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

ENH: template indicators with jinja #1120

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

bzah
Copy link
Contributor

@bzah bzah commented Jun 29, 2022

Pull Request Checklist:

What kind of change does this PR introduce?

Follow up to discussions in here: #1093
This pr is a proof of concept to get an idea of how jinja can be used to format indicator descriptions.

Does this PR introduce a breaking change?

No, it should not.

Other information:

To illustrate the results with heat_wave_frequency:

ds = xr.open_dataset("/Users/aoun/workspace/xclim/climpact.sampledata.gridded.1991-2010.nc")
res = xclim.atmos.heat_wave_frequency(tasmin = ds.tmin,tasmax=ds.tmax)
print(res.attrs["description"])

prints
"Ys number of heat wave events over a given period. an event occurs when the minimum and maximum daily temperature both exceeds specific thresholds : (tmin > 22.0 degc and tmax > 30 degc)over a minimum number of days (3)."

@bzah bzah requested a review from aulemahal June 29, 2022 17:10
@@ -77,7 +81,8 @@ def format(self, format_string: str, /, *args: Any, **kwargs: dict) -> str:
for k, v in DEFAULT_FORMAT_PARAMS.items():
if k not in kwargs:
kwargs.update({k: v})
return super().format(format_string, *args, **kwargs)
kwargs["np"] = np # noqa
Copy link
Contributor Author

Choose a reason for hiding this comment

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

A bit ugly, maybe there is a better way to add numpy in jinja rendering namespace

"An event occurs when the minimum and maximum daily "
"temperature both exceeds specific thresholds : "
"(Tmin > {thresh_tasmin} and Tmax > {thresh_tasmax}) "
"over a minimum number of days ({window}).",
"{% if np.isscalar(thresh_tasmin) and np.isscalar(thresh_tasmax)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is the template for heat_wave_frequency

@bzah
Copy link
Contributor Author

bzah commented Jun 30, 2022

I tried to mitigate the changes by applying both AttrFormatter and jinja templating.
The effects are

  • jinja control structures are interpreted, ignoring values surrounded by single curly brackets.
  • values surrounded by single curly brackets are formatted as usual. (e.g. {freq} -> "Annual")

So, this

res = xclim.atmos.heat_wave_frequency(tasmin=ds.tmin,
                                      tasmax=ds.tmax,
                                      thresh_tasmin=ds.tmin.isel(time=0),
                                      thresh_tasmax=ds.tmax.isel(time=0))
print(res.attrs["description"])

prints

Annual number of heat wave events over a given period. an event occurs when the minimum and maximum daily 
temperature both exceeds specific thresholds : 
(tmin > per_gridcell_tmin_thresholds and tmax > per_gridcell_tmax_thresholds) over a minimum number of days (3).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.006%) to 91.377% when pulling 1a596f7 on enh/add_jinja_for_indicator_templating into 4d5dd03 on master.

@bzah
Copy link
Contributor Author

bzah commented Jul 6, 2022

For the record, I'm working (in icclim) on generic indicators which would fully utilize jinja templating to generate rich metadata.
I hope it won't be too hard for me to upstream this work into xclim once it's in a stable state.

@bzah
Copy link
Contributor Author

bzah commented Sep 23, 2022

Unfortunately, I don't think I will be able to work much more on this PR (or generic indicators in xclim), my contract at cerfacs ends soon and I don't know yet if I will continue working on climate indices afterward.
So feel free to take over or close this PR!
I could present you what I did in icc in case you want port it to xclim though.

@huard
Copy link
Collaborator

huard commented Sep 23, 2022

Hi Abel, sorry to hear that, we really enjoyed your contributions to the project.

Yes, it would be useful for us to get a sense of where things stand so we can work on the xclim port. Let us know when is a good time for you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants