Check for path-like objects rather than Path type, use os.fspath

[mwtoews]

This PR generally changes (e.g.) isinstance(filename, pathlib.Path) to isinstance(filename, os.PathLike), and uses os.fspath to convert it to (usually) str type.

(If it is vital these are always str, then should os.fsdecode be considered? bytes paths are not common, and only possible on some platforms).

If other path-like objects are used e.g. py.path used by the tmpdir pytest fixture, an error message is shown:

ValueError: can only read bytes or file-like objects with engine='scipy' or 'h5netcdf'

This PR allows other path-like objects to be used.

A few typing objects are also adjusted too.

---

Be aware there are file-like and path-like object terms used in the core Python glossary. In light of this, some "file-like" wordings may need to be adjusted, such as the error message described above. This can be done in this PR if anyone aggrees.

[pep8speaks]

Hello @mwtoews! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-10-27 22:38:04 UTC

[github-actions]

Unit Test Results

6 files 6 suites 54m 54s ⏱️
16 210 tests 14 474 ✔️ 1 736 💤 0 ❌
90 456 runs 82 276 ✔️ 8 180 💤 0 ❌

Results for commit 88589cb.

♻️ This comment has been updated with latest results.

[max-sixty]

Thanks a lot @mwtoews !

Would you like to add a note to whatsnew?

Re the path-like vs file-like, that’d be great to clarify in the docs. It’s fine to do this in another PR if you prefer.

[mwtoews]

@max-sixty whats-new entry added, check to see if the paragraph is ok. I'll hold off clarifying file-like vs path-like in the docs for now, but will consider a doc intersphinx link at some time.

[max-sixty]

Thanks @mwtoews !

[gjoseph92]

Note that isinstance(fsspec.OpenFile(...), os.PathLike) due to the magic of ABCs. Are we sure that we want to be calling os.fspath on fsspec files? In many cases (like an S3File, GCSFile, etc.) this will fail with a confusing error like 'S3File' object has no attribute '__fspath__'.

cc @martindurant

[martindurant]

isinstance(X, os.PathLike) is very like hasattr(X, __fspath__) because of:

@classmethoddef__subclasshook__(cls, subclass):
ifclsisPathLike:
return_check_methods(subclass, '__fspath__')
returnNotImplemented

[max-sixty]

@gjoseph92 I'm less informed than most on this — do you have an example of a case that is now confusing? Thank you!

[gjoseph92]

@martindurant exactly, os.PathLike just uses duck-typing, which fsspec matches.

This generally means you can't pass s3fs/gcsfs files into xr.open_dataset (from what I've tried so far). (I don't know if you actually should be able to do this, but regardless, the error would be very confusing to a new user.)

In [32]: xr.open_dataset("s3://noaa-nwm-retrospective-2-1-zarr-pds/lakeout.zarr", engine="zarr")
Out[32]: <xarray.Dataset>Dimensions: (feature_id: 5783, time: 367439)
Coordinates:
*feature_id (feature_id) int32491531747 ... 9470702041021092845latitude (feature_id) float32 ...
longitude (feature_id) float32 ...
*time (time) datetime64[ns] 1979-02-01T01:00:00 ... 2020-12-31T...
Datavariables:
crs|S1 ...
inflow (time, feature_id) float64 ...
outflow (time, feature_id) float64 ...
water_sfc_elev (time, feature_id) float32 ...
Attributes:
Conventions: CF-1.6TITLE: OUTPUTFROMWRF-Hydrov5.2.0-beta2code_version: v5.2.0-beta2featureType: timeSeriesmodel_configuration: retrospectivemodel_output_type: reservoirproj4: +proj=lcc+units=m+a=6370000.0+b=6370000....
reservoir_assimilated_value: Assimilationnotperformedreservoir_type: 1=levelpooleverywherestation_dimension: lake_idIn [33]: xr.open_dataset(fsspec.open("s3://noaa-nwm-retrospective-2-1-zarr-pds/lakeout.zarr"), engine="zarr")
---------------------------------------------------------------------------KeyErrorTraceback (mostrecentcalllast)
<ipython-input-33-76e10d75e2c2>in<module>---->1xr.open_dataset(fsspec.open("s3://noaa-nwm-retrospective-2-1-zarr-pds/lakeout.zarr"), engine="zarr")
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/api.pyinopen_dataset(filename_or_obj, engine, chunks, cache, decode_cf, mask_and_scale, decode_times, decode_timedelta, use_cftime, concat_characters, decode_coords, drop_variables, backend_kwargs, *args, **kwargs)
493494overwrite_encoded_chunks=kwargs.pop("overwrite_encoded_chunks", None)
-->495backend_ds=backend.open_dataset(
496filename_or_obj,
497drop_variables=drop_variables,
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/zarr.pyinopen_dataset(self, filename_or_obj, mask_and_scale, decode_times, concat_characters, decode_coords, drop_variables, use_cftime, decode_timedelta, group, mode, synchronizer, consolidated, chunk_store, storage_options, stacklevel)
797 ):
798-->799filename_or_obj=_normalize_path(filename_or_obj)
800store=ZarrStore.open_group(
801filename_or_obj,
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/common.pyin_normalize_path(path)
21def_normalize_path(path):
22ifisinstance(path, os.PathLike):
--->23path=os.fspath(path)
2425ifisinstance(path, str) andnotis_remote_uri(path):
~/dev/dask-playground/env/lib/python3.9/site-packages/fsspec/core.pyin__fspath__(self)
96def__fspath__(self):
97# may raise if cannot be resolved to local file--->98returnself.open().__fspath__()
99100def__enter__(self):
~/dev/dask-playground/env/lib/python3.9/site-packages/fsspec/core.pyinopen(self)
138beendeleted; butawith-contextisbetterstyle.
139 """
--> 140 out = self.__enter__()
141 closer = out.close
142 fobjects = self.fobjects.copy()[:-1]
~/dev/dask-playground/env/lib/python3.9/site-packages/fsspec/core.py in __enter__(self)
101 mode = self.mode.replace("t", "").replace("b", "") + "b"
102 --> 103 f = self.fs.open(self.path, mode=mode)
104 105 self.fobjects = [f]
~/dev/dask-playground/env/lib/python3.9/site-packages/fsspec/spec.py in open(self, path, mode, block_size, cache_options, compression, **kwargs)
1007 else:
1008 ac = kwargs.pop("autocommit", not self._intrans)
-> 1009 f = self._open(
1010 path,
1011 mode=mode,
~/dev/dask-playground/env/lib/python3.9/site-packages/s3fs/core.py in _open(self, path, mode, block_size, acl, version_id, fill_cache, cache_type, autocommit, requester_pays, **kwargs)
532 cache_type = self.default_cache_type
533 --> 534 return S3File(
535 self,
536 path,
~/dev/dask-playground/env/lib/python3.9/site-packages/s3fs/core.py in __init__(self, s3, path, mode, block_size, acl, version_id, fill_cache, s3_additional_kwargs, autocommit, cache_type, requester_pays)
1824 1825 if "r" in mode:
-> 1826 self.req_kw["IfMatch"] = self.details["ETag"]
1827 1828 def _call_s3(self, method, *kwarglist, **kwargs):
KeyError: 'ETag'

[max-sixty]

Thanks @gjoseph92, that makes sense.

Do you know whether there's a standard approach that works for these? I would expect xarray's needs are fairly standard for this function of "take something that's path-like".

[martindurant]

"s3://noaa-nwm-retrospective-2-1-zarr-pds/lakeout.zarr" is a directory, right? You cannot open that as a file, or maybe there is no equivalent key at all (because s3 is magic like that).
No, you should not be able to do this directly - zarr requires a path which fsspec can turn into a mapper, or an instantiated mapper.

To make a bare mapper (i.e., dict-like):

m = fsspec.get_mapper("s3://noaa-nwm-retrospective-2-1-zarr-pds/lakeout.zarr", ...)

or you could use zarr's FSMapper meant specifically for this job.

[gjoseph92]

"s3://noaa-nwm-retrospective-2-1-zarr-pds/lakeout.zarr" is a directory, right? You cannot open that as a file

Yeah correct. I oversimplified this from the problem I actually cared about, since of course zarr is not a single file that can be fsspec.open'd in the first place, and the zarr engine is doing some magic there when passed the plain string.

Here's a more illustrative example:

In [1]: importxarrayasxrIn [2]: importfsspecIn [3]: importosIn [4]: url="s3://noaa-nwm-retrospective-2-1-pds/model_output/1979/197902010100.CHRTOUT_DOMAIN1.comp"# a netCDF file in s3In [5]: f=fsspec.open(url)
In [6]: fOut[6]: <OpenFile'noaa-nwm-retrospective-2-1-pds/model_output/1979/197902010100.CHRTOUT_DOMAIN1.comp'>In [7]: isinstance(f, os.PathLike)
Out[7]: TrueIn [8]: s3f=f.open()
In [9]: s3fOut[9]: <File-likeobjectS3FileSystem, noaa-nwm-retrospective-2-1-pds/model_output/1979/197902010100.CHRTOUT_DOMAIN1.comp>In [10]: isinstance(s3f, os.PathLike)
Out[10]: FalseIn [11]: ds=xr.open_dataset(s3f, engine='h5netcdf')
In [12]: dsOut[12]: <xarray.Dataset>Dimensions: (time: 1, reference_time: 1, feature_id: 2776738)
Coordinates:
*time (time) datetime64[ns] 1979-02-01T01:00:00*reference_time (reference_time) datetime64[ns] 1979-02-01*feature_id (feature_id) int32101179181 ... 11800018031180001804latitude (feature_id) float32 ...
longitude (feature_id) float32 ...
Datavariables:
crs|S1 ...
order (feature_id) int32 ...
elevation (feature_id) float32 ...
streamflow (feature_id) float64 ...
q_lateral (feature_id) float64 ...
velocity (feature_id) float64 ...
qSfcLatRunoff (feature_id) float64 ...
qBucket (feature_id) float64 ...
qBtmVertRunoff (feature_id) float64 ...
Attributes: (12/18)
TITLE: OUTPUTFROMWRF-Hydrov5.2.0-beta2featureType: timeSeriesproj4: +proj=lcc+units=m+a=6370000.0+b=6370000.0 ...
model_initialization_time: 1979-02-01_00:00:00station_dimension: feature_idmodel_output_valid_time: 1979-02-01_01:00:00
... ...
model_configuration: retrospectivedev_OVRTSWCRT: 1dev_NOAH_TIMESTEP: 3600dev_channel_only: 0dev_channelBucket_only: 0dev: dev_prefixindicatesdevelopment/internalme...
In [13]: ds=xr.open_dataset(f, engine='h5netcdf')
---------------------------------------------------------------------------AttributeErrorTraceback (mostrecentcalllast)
<ipython-input-13-de834ca911b4>in<module>---->1ds=xr.open_dataset(f, engine='h5netcdf')
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/api.pyinopen_dataset(filename_or_obj, engine, chunks, cache, decode_cf, mask_and_scale, decode_times, decode_timedelta, use_cftime, concat_characters, decode_coords, drop_variables, backend_kwargs, *args, **kwargs)
493494overwrite_encoded_chunks=kwargs.pop("overwrite_encoded_chunks", None)
-->495backend_ds=backend.open_dataset(
496filename_or_obj,
497drop_variables=drop_variables,
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/h5netcdf_.pyinopen_dataset(self, filename_or_obj, mask_and_scale, decode_times, concat_characters, decode_coords, drop_variables, use_cftime, decode_timedelta, format, group, lock, invalid_netcdf, phony_dims, decode_vlen_strings)
384 ):
385-->386filename_or_obj=_normalize_path(filename_or_obj)
387store=H5NetCDFStore.open(
388filename_or_obj,
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/common.pyin_normalize_path(path)
21def_normalize_path(path):
22ifisinstance(path, os.PathLike):
--->23path=os.fspath(path)
2425ifisinstance(path, str) andnotis_remote_uri(path):
~/dev/dask-playground/env/lib/python3.9/site-packages/fsspec/core.pyin__fspath__(self)
96def__fspath__(self):
97# may raise if cannot be resolved to local file--->98returnself.open().__fspath__()
99100def__enter__(self):
AttributeError: 'S3File'objecthasnoattribute'__fspath__'

Because the plain fsspec.OpenFile object has an __fspath__ attribute (but calling it raises an error), it causes xarray.backends.common._normalize_path to fail.

Because the s3fs.S3File object does not have an __fspath__ attribute, normalize_path doesn't try to call os.fspath on it, so the file-like object is able to be passed all the way down into h5netcdf, which is able to handle it.

Note though that if I downgrade xarray to 0.19.0 (last version before this PR was merged), I still can't use the plain `fssspec.OpenFile` object successfully. It's not xarray's fault anymore—it gets passed all the way into h5netcdf—but h5netcdf also tries to call `fspath` on the `OpenFile`, which fails in the same way.

In [1]: importxarrayasxrIn [2]: importfsspecIn [3]: xr.__version__Out[3]: '0.19.0'In [4]: url="s3://noaa-nwm-retrospective-2-1-pds/model_output/1979/197902010100.CHRTOUT_DOMAIN1.comp"# a netCDF file in s3In [5]: f=fsspec.open(url)
In [6]: xr.open_dataset(f.open(), engine="h5netcdf")
Out[6]: <xarray.Dataset>Dimensions: (time: 1, reference_time: 1, feature_id: 2776738)
Coordinates:
*time (time) datetime64[ns] 1979-02-01T01:00:00*reference_time (reference_time) datetime64[ns] 1979-02-01*feature_id (feature_id) int32101179181 ... 11800018031180001804latitude (feature_id) float32 ...
longitude (feature_id) float32 ...
Datavariables:
crs|S1 ...
order (feature_id) int32 ...
elevation (feature_id) float32 ...
streamflow (feature_id) float64 ...
q_lateral (feature_id) float64 ...
velocity (feature_id) float64 ...
qSfcLatRunoff (feature_id) float64 ...
qBucket (feature_id) float64 ...
qBtmVertRunoff (feature_id) float64 ...
Attributes: (12/18)
TITLE: OUTPUTFROMWRF-Hydrov5.2.0-beta2featureType: timeSeriesproj4: +proj=lcc+units=m+a=6370000.0+b=6370000.0 ...
model_initialization_time: 1979-02-01_00:00:00station_dimension: feature_idmodel_output_valid_time: 1979-02-01_01:00:00
... ...
model_configuration: retrospectivedev_OVRTSWCRT: 1dev_NOAH_TIMESTEP: 3600dev_channel_only: 0dev_channelBucket_only: 0dev: dev_prefixindicatesdevelopment/internalme...
In [7]: xr.open_dataset(f, engine="h5netcdf")
---------------------------------------------------------------------------KeyErrorTraceback (mostrecentcalllast)
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/file_manager.pyin_acquire_with_cache_info(self, needs_lock)
198try:
-->199file=self._cache[self._key]
200exceptKeyError:
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/lru_cache.pyin__getitem__(self, key)
52withself._lock:
--->53value=self._cache[key]
54self._cache.move_to_end(key)
KeyError: [<class'h5netcdf.core.File'>, (<OpenFile'noaa-nwm-retrospective-2-1-pds/model_output/1979/197902010100.CHRTOUT_DOMAIN1.comp'>,), 'r', (('decode_vlen_strings', True), ('invalid_netcdf', None))]
Duringhandlingoftheaboveexception, anotherexceptionoccurred:
AttributeErrorTraceback (mostrecentcalllast)
<ipython-input-7-e6098b8ab402>in<module>---->1xr.open_dataset(f, engine="h5netcdf")
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/api.pyinopen_dataset(filename_or_obj, engine, chunks, cache, decode_cf, mask_and_scale, decode_times, decode_timedelta, use_cftime, concat_characters, decode_coords, drop_variables, backend_kwargs, *args, **kwargs)
495496overwrite_encoded_chunks=kwargs.pop("overwrite_encoded_chunks", None)
-->497backend_ds=backend.open_dataset(
498filename_or_obj,
499drop_variables=drop_variables,
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/h5netcdf_.pyinopen_dataset(self, filename_or_obj, mask_and_scale, decode_times, concat_characters, decode_coords, drop_variables, use_cftime, decode_timedelta, format, group, lock, invalid_netcdf, phony_dims, decode_vlen_strings)
372373filename_or_obj=_normalize_path(filename_or_obj)
-->374store=H5NetCDFStore.open(
375filename_or_obj,
376format=format,
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/h5netcdf_.pyinopen(cls, filename, mode, format, group, lock, autoclose, invalid_netcdf, phony_dims, decode_vlen_strings)
176177manager=CachingFileManager(h5netcdf.File, filename, mode=mode, kwargs=kwargs)
-->178returncls(manager, group=group, mode=mode, lock=lock, autoclose=autoclose)
179180def_acquire(self, needs_lock=True):
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/h5netcdf_.pyin__init__(self, manager, group, mode, lock, autoclose)
121# todo: utilizing find_root_and_group seems a bit clunky122# making filename available on h5netcdf.Group seems better-->123self._filename=find_root_and_group(self.ds)[0].filename124self.is_remote=is_remote_uri(self._filename)
125self.lock=ensure_lock(lock)
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/h5netcdf_.pyinds(self)
187 @property188defds(self):
-->189returnself._acquire()
190191defopen_store_variable(self, name, var):
~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/h5netcdf_.pyin_acquire(self, needs_lock)
179180def_acquire(self, needs_lock=True):
-->181withself._manager.acquire_context(needs_lock) asroot:
182ds=_nc4_require_group(
183root, self._group, self._mode, create_group=_h5netcdf_create_group~/.pyenv/versions/3.9.1/lib/python3.9/contextlib.pyin__enter__(self)
115delself.args, self.kwds, self.func116try:
-->117returnnext(self.gen)
118exceptStopIteration:
119raiseRuntimeError("generator didn't yield") fromNone~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/file_manager.pyinacquire_context(self, needs_lock)
185defacquire_context(self, needs_lock=True):
186"""Context manager for acquiring a file."""-->187file, cached=self._acquire_with_cache_info(needs_lock)
188try:
189yieldfile~/dev/dask-playground/env/lib/python3.9/site-packages/xarray/backends/file_manager.pyin_acquire_with_cache_info(self, needs_lock)
203kwargs=kwargs.copy()
204kwargs["mode"] =self._mode-->205file=self._opener(*self._args, **kwargs)
206ifself._mode=="w":
207# ensure file doesn't get overriden when opened again~/dev/dask-playground/env/lib/python3.9/site-packages/h5netcdf/core.pyin__init__(self, path, mode, invalid_netcdf, phony_dims, **kwargs)
978self._preexisting_file=modein {"r", "r+", "a"}
979self._h5py=h5py-->980self._h5file=self._h5py.File(
981path, mode, track_order=track_order, **kwargs982 )
~/dev/dask-playground/env/lib/python3.9/site-packages/h5py/_hl/files.pyin__init__(self, name, mode, driver, libver, userblock_size, swmr, rdcc_nslots, rdcc_nbytes, rdcc_w0, track_order, fs_strategy, fs_persist, fs_threshold, fs_page_size, page_buf_size, min_meta_keep, min_raw_keep, locking, **kwds)
484name=repr(name).encode('ASCII', 'replace')
485else:
-->486name=filename_encode(name)
487488iftrack_orderisNone:
~/dev/dask-playground/env/lib/python3.9/site-packages/h5py/_hl/compat.pyinfilename_encode(filename)
17filenamesinh5pyformoreinformation.
18 """
---> 19 filename = fspath(filename)
20 if sys.platform == "win32":
21ifisinstance(filename, str):
~/dev/dask-playground/env/lib/python3.9/site-packages/fsspec/core.pyin__fspath__(self)
96def__fspath__(self):
97# may raise if cannot be resolved to local file--->98returnself.open().__fspath__()
99100def__enter__(self):
AttributeError: 'S3File'objecthasnoattribute'__fspath__'

The problem is that OpenFile doesn't have a read or seek method, so h5py doesn't think it's a proper file-like object and tries to fspath it here: https://github.com/h5py/h5py/blob/master/h5py/_hl/files.py#L509

So I may just be misunderstanding what an fsspec.OpenFile object is supposed to be (it's not actually a file-like object until you .open() it?). But I expect users would be similarly confused by this distinction.

[martindurant]

OK, I get you - so the real problem is that OpenFile can look path-like, but isn't really.

OpenFile is really a file-like factory, a proxy for open file-likes that you can make (and seialise for Dask). Its main purpose is to be used in a context:

withfsspec.open(url) asf:
ds=xr.open_dataset(f, engine="h5netcdf")

except that the problem with xarray is that it will want to keep this thing open for subsequent operations, so you either need to put all that in the context, or use .open()/.close() as you have been.

[gjoseph92]

Yeah, I guess I expected OpenFile to, well, act like an open file. So maybe this is more of an fsspec interface issue?

I'll open a separate issue for improving the UX of this in xarray though. I think this would be rather confusing for new users.