issue_comments

Thank you for this refactor, @keewis!

pooch writes to a temp file and then moves it into place, which is probably a sufficiently robust solution for that.

I concur. 👍🏽

data versioning might be useful if we ever decide to change datasets: once we do, the examples would change as well, so people might not be able to exactly reproduce them. Not sure how much of an issue that is, though.

I fixed the failing tests, but the trick I used (point $XDG_CACHE_DIR to a temporary directory) only works on linux.

I suspect we could make Pooch work OK by tagging releases in the xarray-data repository. Then we can manually update the version number inside xarray whenever we add or update a dataset.

I don't think it's important to preserve the GitHub URL and branch options. I suspect those options exist strictly for testing new datasets.

I'm surprised that Pooch seems to be focused on data that is bundled in code repositories. It seems like that could easily lead to bloated repositories (though I guess xarray data is bigger than image data, for example).

Ping @andersy005 who has experience with pooch

I switched the tutorial code to use pooch (the tests still need to be updated), which means that we get the appdirs, download, and caching for free. It is a optional dependency for now, thus requiring pooch to be able to use xr.tutorial.open_*.

However, pooch seems to be mainly written for repositories that contain both the data and the code (like scikit-image's data module). If I understand it correctly, this means that we can't take full advantage of the features pooch has (such as the file registry, or the version mechanism). Some of the options we have (github_url and branch) make this even more difficult. I wonder if we should deprecate / remove them?

So for now, instead of using pooch.create(base_url=url, registry=registry).fetch(name) we use pooch.retrieve(url, hash=None, ...). Not sure if there's a better way, though.

what should we do with this? Maybe it's easier to decide if we split this PR into one that introduces the caching and a new one that tries to find the correct "appdirs" path (which I think is nice to have but not really important)?

I vendored appdirs.user_cache_dir, but I need help with mypy: the windows code uses ctypes.windll which is only available on windows

Let's skip this module for mypy. You can add it to the list in setup.cfg.

I vendored appdirs.user_cache_dir, but I need help with mypy: the windows code uses ctypes.windll which is only available on windows

Edit: appdirs solves that (but: it's a new dependency)

See #2815 :)

As mentioned in that issue, we could definitely vendor a copy of appdirs if desired. Or we could pick some simple heuristic. In practice I think ~/.cache is fine. It's only standard on Linux, but I have a handful of directories there on my Mac laptop already.

thanks for pointing me to that issue, @dcherian

Edit: appdirs solves that (but: it's a new dependency)

See #2815 :)

I would suggest just creating the full directory tree, e.g., with os.makedirs(path, exist_ok=True). I don't think we want to let this be inconsistent.

When writing this I wanted to make sure we didn't force a ~/.cache folder on everyone, regardless of the platform.

After looking into this a bit more, it seems we would probably need to have different cache folders for different platforms: - on windows, the path is given by APPDATA (or LOCALAPPDATA, not sure) (I can't find a official resource for this) - on macos it can be either ~/Library/Caches (or ~/Library/Application Support?) - on unix (following the freedesktop basedir spec) it is indicated by XDG_CACHE_HOME with a default of ~/.cache.

So a currently released version of xarray will always put it into ~/.xarray_tutorial_data, and my changes will partially align with the freedesktop spec.

How closely should we follow those specifications?

Edit: appdirs solves that (but: it's a new dependency)

Edit: I also changed the default cache directory to ~/.cache/xarray_tutorial_data with a fallback to the old default if ~/.cache does not exist

I would suggest just creating the full directory tree, e.g., with os.makedirs(path, exist_ok=True). I don't think we want to let this be inconsistent.

using a existing library for that instead of writing our own caching / downloading functions does seem useful (the code there seems to be much more sophisticated than anything in the tutorial module), but we'd be introducing a new dependency: not sure how much of an issue that would be.

We could also make it a optional dependency, but that would mean that the tutorial module is not always available.

For these kind of cached files, would it be worth using something like Pooch for xarray?