cf-xarray icon indicating copy to clipboard operation
cf-xarray copied to clipboard

Cache flag-related objects and refactor extraction of flags

Open Descanonge opened this issue 1 year ago • 2 comments

The dictionary of flag values/masks is cached in the CFAccessor, moving the code of create_flag_dict into a property of the accessor.

The dataset of booleans masks is cached in the CFDataArrayAccessor. I don't know if this is a welcomed change. I would argue that if it is presented as a property (.flags), I would expect it to be the same instance on different calls, contrary to a method like .flags() or .get_flags(). But I'm not very familiar with the rest of the cf-xarray API. Anyway it can be reverted easily.

I have also refactored the construction of that flag dataset to make it (hopefully) more readable and understable. It makes use of the FlagParams named-tuple. The operations on the data are unchanged.

Descanonge avatar Jun 25 '24 16:06 Descanonge

I have also refactored the construction of that flag dataset to make it (hopefully) more readable and understable.

Nice!

Re: API if it is useful to get flag_dict then perhaps we should create a Flags dataclass with as_dict and as_dataset methods?

dcherian avatar Jun 25 '24 21:06 dcherian

Re: API if it is useful to get flag_dict then perhaps we should create a Flags dataclass with as_dict and as_dataset methods?

I am not sure I'm following. Something like datarray.cf.flags.as_dict ? It would regroup the code for flags, which is nice. But as_dataset needs to check that the object is an array and not a Dataset (or maybe select a valid flag variable in the Dataset if there is only one). It could essentially do create_flag_dict on init, so that would be cached, and making the flag dataset available from a method seems reasonable.

Descanonge avatar Jun 25 '24 22:06 Descanonge