Removed deprecated functions, deprecate testing.open_dataset
#2139
Conversation
| @@ -69,13 +69,12 @@ def open_dataset(nimbus): # numpydoc ignore=PR01 | |||
| """ | |||
|
|
|||
| def _open_session_scoped_file(file: str | os.PathLike, **xr_kwargs): | |||
| nimbus_kwargs = dict(branch=TESTDATA_BRANCH, repo=TESTDATA_REPO_URL, cache_dir=nimbus.path) | |||
There was a problem hiding this comment.
didn't you change the conventions on how to define dict? aren't we suppose to use {} exclusively and not dict?
There was a problem hiding this comment.
Right. I think I went for readability here, but let's be consistent.
| file = tmp_path / "test_polydetrend.nc" | ||
| fx.ds.to_netcdf(file, engine="h5netcdf") | ||
|
|
||
| ds = open_dataset(file) | ||
| ds = xr.open_dataset(file) |
There was a problem hiding this comment.
so the tmp_path function differently, they don't need nimbus?
There was a problem hiding this comment.
where exactly are they stored anyways?
There was a problem hiding this comment.
Good question! It depends on how the tests are launched:
- If the tests are launched with
numprocesses(no distributed testing), the files are found inXDG_CACHE_HOME(or equivalent for macOS/Windows). - If the tests are launched with parallel execution, the first worker able to start downloading test data places the data into
XDG_CACHE_HOME(if not already there) and afterwards, all workers copy this folder to a new temporary location. This prevents synonymous reads of shared files by multiple workers (which cause problems forpython-NetCDF4).
There was a problem hiding this comment.
Woops, hold on. In this case, yes, the tmp_path is worker-dependent. nimbus is used for resolving dataset locations that identify true testing data. Within a test, the temp filepath is a full location, self-contained to that test * worker.
Pull Request Checklist:
number) and pull request (:pull:number) has been addedWhat kind of change does this PR introduce?
sfcwind_2_uas_vasanduas_vas_2_sfcwindconverter functions.xclim.testing.open_datasetto be removed in a future versionDoes this PR introduce a breaking change?
Yes. Functions that were previously deprecated have been removed. Suggestions have been made to use the
nimbusclass to manage fetching testing data.