issue_comments

CI failures are all the issue with dask distributed (https://github.com/pydata/xarray/issues/1540), so I'm going ahead and merging.

Thanks @willirath !

Thanks @shoyer!

I like the shorter example. It shows the essence of why pathlib is such a nice thing.

Is there anything more to do in this PR?

Am 31. August 2017 17:48:24 schrieb Stephan Hoyer notifications@github.com:

shoyer approved this pull request.

@@ -21,9 +21,38 @@ v0.9.7 (unreleased) Enhancements ~~~~~~~~~~~~

+- Support for pathlib.Path objects added to

I just pushed a commit to add this

• .. ipython::
• :verbatim:
• In [1]: import xarray as xr +
• In [2]: from pathlib import Path # In Python 2, use pathlib2! +
• In [3]: data_dir = Path("data/") +
• In [4]: one_file = data_dir / "dta_for_month_01.nc" +
• In [5]: print(xr.open_dataset(one_file))
• Out[5]:
• <xarray.Dataset>
• [...] +
• In [6]: all_files = data_dir.glob("dta_for_month_*.nc")

I removed this example from What's New for two reason: 1. The section was getting a little longer than essential 2. It's actually a bit of an anti-pattern, since the order of paths matters to xarray but is arbtirary from glob. The right way to write this is sorted(data_dir.glob(...)).

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/pydata/xarray/pull/1514#pullrequestreview-59903100

AppVeyor test failed with HTTP time-outs when trying to get repodata.json. Can somebody trigger the build again?

flake8 now fails because of the unused pathlib in xarray/tests/__init__.py.

And then I remove the pathlib2 dependency I've introduced in setup.py again?

Yes, pathlib2 should not be a required dependency.

take a look at what we do for handling the optional dask dependency

Ah, that's nice!

And then I remove the pathlib2 dependency I've introduced in setup.py again?

@willirath take a look at what we do for handling the optional dask dependency: https://github.com/pydata/xarray/blob/bcd608101133c0cb84c74d341d22edef71ef4818/xarray/core/pycompat.py#L55-L60

One more thing:

Can we add pathlib2 as an optional dependency and handle the case where it's not installed?

Currently, I've set pathlib2 as a mandatory requirement for Python 2. How'd we achieve pathlib being optional? Test for pathlib upon import and wrap all the if isinstace(path, Path): path = str(path) in a function just passing unmodified path if pathlib's not present?

• [x] Tests added / passed

Tests are already done on my personal Travis Account (skipped intermediate commits there).

• [x] Passes git diff upstream/master | flake8 --diff

Actually true only for git diff upstream/master xarray/ | flake8 --diff. But I guess that's what should pass the linter?

To me, aae32a8 looks like pathlib support is now present whereever it makes sense.

Thanks for the review @shoyer !

On my machine, I've already stared adapting the other backends (to_netcdf et al) to support pathlib as well. I'll include it here.

Curently, tests with Python 2 are failing, because there is no explicit pathlib dependency yet.

I missed this, apologies.

Can we add pathlib2 as an optional dependency and handle the case where it's not installed?

Are you sure pathlib exists in py2? I had thought you needed to install pathlib2 (I may be wrong on the specifics)