-
Notifications
You must be signed in to change notification settings - Fork 59
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
Support for temperature differences in CF attributes #1836
Conversation
…nvert_units_to. Add test and docs.
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
@juliettelavoie @aulemahal Ce PR va possiblement affecter des fonctions que vous avez créées. |
…ng metadata to non temperature variables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
…t for non-temperature variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall this looks good!
However, should we ensure that all indicators that give temperatures have the proper units_metadata
? For example :
xc.atmos.daily_temperature_range(ds=ds)
# Attributes:
# units: K
# units_metadata: temperature: unknown
And while testing this I realized that:
xc.indices.daily_temperature_range(tasmin=tn, tasmax=tx)
#Attributes:
# units: delta_degC
Which isn't great as it's not CF. This problem predates your PR, but it would be nice to apply your changes to this indice!
Set
units_metadata
attribute to"temperature: difference"
for arrays representing a difference between two temperatures.Pull Request Checklist:
units_metadata
where appropriate. #1822number
) and pull request (:pull:number
) has been addedWhat kind of change does this PR introduce?
units_metadata
attribute set to"temperature: difference"
whenever values represent a difference between two temperatures (degree days, diurnal amplitude, bias, etc).delta_degC
, which is not CF compliant, by the original input DataArray units and theunits_metadata
attribute.pint2cfattrs
to convert delta units into a dictionary of attributes, includingunits_metadata
.Does this PR introduce a breaking change?
Yes, units returned by some indices will change.
Other information:
I suspect I missed some instances where fixes are needed.