Update xcdat_open() #1212
Update xcdat_open() #1212
Conversation
|
@lee1043 This is low priority to review. For testing I've run Demos 1b, 2b, 4, and 5 as it looks like those drivers use the xcdat_open() function. |
There was a problem hiding this comment.
Looks good to me. Also checked with demo 4 notebook which ran without any issue. Thanks for the PR, @acordonez!
|
|
||
| def xcdat_open( | ||
| infile: Union[str, list], data_var: str = None, decode_times: bool = True | ||
| infile: Union[str, list], data_var: str = None, decode_times: bool = True, chunks={} |
There was a problem hiding this comment.
@acordonez thank you for the PR! It looks good to me in general.
This may not a big deal but I wonder if this could be chunks=None to be consistent to the default of underline function that is xarray.open_mfdataset
There was a problem hiding this comment.
@lee1043 The extremes and drcdm metrics need to be able to specify the chunks to ensure that the time axis is continuous across a single chunk.
There was a problem hiding this comment.
@lee1043 Would chunks=None mean that no chunking is used by default? That might be fine?
There was a problem hiding this comment.
@lee1043 The extremes and drcdm metrics need to be able to specify the chunks to ensure that the time axis is continuous across a single chunk.
That sounds like a good reason to keep the PR as it is. Thanks for the comment!
There was a problem hiding this comment.
@acordonez I just found that chunks={} as default causes error in modes of variability code by opening dataset with dask chunks. I can make modes of variability as a special case by having chunks=None when using xcdat_open but haven't tested other metrics. If dask chunks are needed for only a few metrics, how about setting the default as chunks=None while in those special cases use xcdat_open with chunks={}?
There was a problem hiding this comment.
@lee1043 That suggestions sounds good to me.
xcdat_open is currently used by the mean climate metrics, the modes of variability metrics, mjo metrics, and monsoon (Sperber).