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

set_options is applied on init, not enter

Open mcgibbon opened this issue 2 years ago • 3 comments

Really nice stuff! I'm not impacted by this, but I noticed a bug. In the following example, I would expect corresponding options to be applied to operations on their corresponding datasets. Currently, as I understand set_options, ds_2_options will be used for both ds_1 calls, and neither context manager will be used for the ds_2 call.

ds_1_options = set_options(...)
ds_2_options = set_options(...)

ds_1_cf.<something>

with ds_1_options:
    ds_1.cf.<something>
    
with ds_2_options:
    ds_2.cf.<something>

I believe the fix would involve writing a test like the above example and moving the init code into enter.

mcgibbon avatar Jan 25 '22 22:01 mcgibbon

Unrelated to the bug, but it would be cool to be able to apply these options persistently to a particular dataset, to avoid needing a with on every operation for that dataset.

mcgibbon avatar Jan 25 '22 22:01 mcgibbon

Ah thanks for the report.

we intended (see https://cf-xarray.readthedocs.io/en/latest/custom-criteria.html )

with set_options(...)
    ds_1.cf.<something>

But I'll look into __enter__ vs __init__.

For persistence, we'd have to store your criteria in ds which gets hairy unfortunately. Do you have a use case?

dcherian avatar Jan 25 '22 22:01 dcherian

For persistence, we'd have to store your criteria in ds which gets hairy unfortunately. Do you have a use case?

For prioritization, I'm not a current user. If this is hard and not generally useful, don't worry about it.

If I were to be a user, we work with datasets from different sources that break CF conventions in different ways. I'd like to be able to specify for each dataset what ways it breaks CF conventions, and not have to worry about potential interactions between dataset configurations if I were to merge all exceptions for all our data sources into one configuration. In other words, there's a one to one map between the exceptions we want to be made and the dataset we're operating on.

A "standardize" method that retrieves a new xarray dataset with everything CF compliant might be a good way to support this use case. I could convert each dataset using a different context manager, and not worry about differences between datasets from that point forward.

mcgibbon avatar Jan 25 '22 22:01 mcgibbon