fix freq/offset/indexer attrs in SPI #1538
Conversation
xclim/indices/stats.py
Outdated
| @@ -660,7 +660,7 @@ def standardized_index_fit_params( | |||
| window: int, | |||
| dist: str, | |||
| method: str, | |||
| offset: Quantified | None = None, | |||
| offset: Quantified = "", | |||
There was a problem hiding this comment.
Disclaimer: I don't really understand what the original issue is.
In indices/_conversion.py, there are a lot of instances of a Quantified parameter set to None. I suspect it would be best to have consistency in how we represent default null Quantified values.
Would doing something like
offset = offset or ""
let you keep None as the default but not mess with attribute records?
There was a problem hiding this comment.
Yes, that's a good way around, I'll adopt this instead, thanks
There was a problem hiding this comment.
The original issue was what you describe, None and {} mess the attributes records, an array can'T be saved in netcdf. So I wanted to change the default values in the code (so that what is used in the code would be the same thing as in attrs), but as you suggest, simply changing was is stored in the attrs but keep using None in the rest of the code is also possible.
There was a problem hiding this comment.
Do we need to do the same with other indices ?
…nto fix_spi_attrs
Pull Request Checklist:
number) and pull request (:pull:number) has been addedWhat kind of change does this PR introduce?
Does this PR introduce a breaking change?
No
Other information: