issue_comments

Sorry, I really don't know what goes inside xarray's cache layers. It seems that fsspec is doing the right thing if it opens via one route, and parallel=True shouldn't require any serialisation for the in-process threaded scheduler.

what are the limitations of the netcdf3 standard vs netcdf4

No compression, encoding or chunking except for the one "append" dimension.

scipy only reads/writes netcdf2/3 ( https://docs.scipy.org/doc/scipy/reference/generated/scipy.io.netcdf_file.html ), which is a very different and simpler format than netcdf4. The latter uses HDF5 as a container, and h5netcdf as the xarray engine. I guess "to_netcdf" is ambiguous.

I use the engine="scipy" one for reading.

This is netCDF3, in that case. If that's fine for you, no problem.

Maybe it is netCDF3? xarray is supposed to be able to determine the file type with fsspec.open("s3://some_bucket/some_remote_destination.nc", mode="rb") as ff: ds = xr.open_dataset(ff) but maybe play with the engine= argument.

I do generally recommend cache_type="first" for reading HDF5 files, because they tend to have most of the metadata in the header area of the file, with short pieces of metadata "elsewhere"; so the default readahead doesn't perform very well.

As to what the two writers might be doing differently, I only have guesses. I imagine xarray leaves it entirely to HDF to make whatever choices it likes. Dask does not write in parallel, since HDF does not support that, but it may order the writes more logically. It does set up the whole set of variables as a initialisation stage before writing any data - I don't know if xarray does this.

Would you mind writing out long-hand the version that worked and the version that didn't?

I can confirm that something like the following does work, basically automating the "write local and then push" workflow: import xarray as xr import fsspec ds = xr.open_dataset('http://geoport.usgs.esipfed.org/thredds/dodsC' '/silt/usgs/Projects/stellwagen/CF-1.6/BUZZ_BAY/2651-A.cdf') outfile = fsspec.open('simplecache::gcs://mdtemp/foo2.nc', mode='wb') with outfile as f: ds.to_netcdf(f)

Unfortunately, directly writing to the remote file without a local cached file is not supported, because HDF5 does not write in a linear way.

I recommend turning on logging in the HTTP file system client = Client(n_workers=1, threads_per_worker=32, memory_limit='64GB') client.run(fsspec.utils.setup_logging, logger_name="fsspec.http") fsspec.utils.setup_logging(logger_name="fsspec.http") and looking for errors

It loos reasonable to me. I'm not sure if the warning is needed or not - we don't expect anyone to see it, or if they do, necessarily do anything about it. It's not unusual for code interacting with a file-like object to move the file pointer.

I agree that is not None would make sense here. We could implement bool() for FSMap, but if we are following the dict API, indeed it should be the same as a test for empty.

@nestabur , if you pass the s3 path directly to xarray/zarr rather than making your own FSMap, you should get a FSStore storage layer (similar but different!). Does this have the same behaviour?

Yes, it is reasonable to always seek(0) or to copy the file. I am not certain why/where xarray is caching the open file, though - I would have thought that a new file instance is made for each open_dataset(). I am not certain whether seeking/reading from the same file in multiple places might have unforeseen consequences, such as when doing open_dataset in multiple threads.

I am mildly against subclassing from RawIOBase, since some file-likes might choose to implement text mode right in the class (as opposed to a text wrapper layered on top). Pretty surprised that it doesn't have read()/write(), though, since all the derived classes do.

Can you please explicitly check the type and __dict__ of fp?

Python's HTTP server does not normally provide content lengths without some extra work, that might be the difference.

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: python with fsspec.open(url) as f: 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.

"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.

isinstance(X, os.PathLike) is very like hasattr(X, __fspath__) because of: python @classmethod def __subclasshook__(cls, subclass): if cls is PathLike: return _check_methods(subclass, '__fspath__') return NotImplemented

It would be interesting to turn on s3fs logging to see the access pattern, if you are interested. python fsspec.utils.setup_logging(logger_name="s3fs") Particularly, I am interested in whether xarray is loading chunk-by chunk serially versus concurrently. It would be good to know your chunksize versus total array size.

The dask version is interesting: xr.open_zarr(lookup(f"{path_forecast}/surface"), chunks={}) # uses dask where the dask partition size will be the same as the underlying chunk size. If you find a lot of latency (small chunks), you can sometimes get an order of magnitude download performance increase by specifying the chunksize along some dimension(s) to be a multiple of the on-disk size. I wouldn't normally recommend Dask just for loading the data into memory, but feel free to experiment.

The conversation here seems to have stalled, but I feel like it was useful. Did we gather any useful actions?

Some thoughts: - fsspec's mapper (or even the filesystem instance) could hold default kwargs to be applied to any open() function, perhaps separate ones for reading and writing. In this case, that would mean supplying acl="public-read". - the consolidate_zarr could be made to accept extra parameters to define how the .zmetadata file is made, perhaps even accept a file-like object to write into, so the user gets complete control - the mapper translates some exceptions into KeyError, which is what zarr needs to conclude that .zmetadata is missing. You could include PermissionError in this now (missing_exceptions kwarg to get_mapper), but we can talk about what the best defaults are.

should we change that?

Perhaps so? We are releasing pretty frequently, though, and if there is a problem here, we'd be happy to put out a bugfix.

There was a release of fsspec, but I don't see why anything would have changed here. Can you see whether the failure is associated with the new version?

I only have vague thoughts.

To be sure: you can pickle the file-system, any mapper (.get_mapper()) and any open file (.open()), right?

The question here is, why msgpack is being invoked. Those items, as well as any internal xarray stuff should only be in tasks, and so pickled. Is there a high-level-graph layer encapsulating things that were previously pickled? The only things that appear in any HLG-layer should be the paths and storage options needed to open a file-system, not the file-system itself.

Unsure whether checking hasattr is better than just trying to read the object and catching an error

Agree, that's fine. An AttributeError in this calling function might look weird, though, so you could have both.

you could read it into BytesIO and pass the BytesIO instance

This is in general a bad idea, since we are wanting to deal with large files, and we have random access capabilities.

Ideally xarray would work with fsspec

It does, but this is an edge case of using fsspec for local files; these are normally passed as the filename.

xref https://github.com/pangeo-forge/pangeo-forge/pull/87

Ping, can I please ask what the current status is here?

Thank you, @dcherian

Can someone please explain the minimum version policy that is failing ``` Package Required Policy Status

aiobotocore 1.1.2 (2020-08-18) 0.12 (2020-02-24) > (!) (w) ```

@raybellwaves , might I paraphrase to "this PR is useful, please merge!" ?

I have decided, on reflection, to back away on the scope here and only implement for zarr for now, since, frankly, I am confused about what should happen for other backends, and they are not tested. Yes, some of them are happy to accept file-like objects, but others either don't do that at all, or want the URL passing through. My code would have changed how things were handled, depending on whether it passed through open_dataset or open_mfdataset. Best would be to set up a set of expectations as tests.

Thanks, @kmuehlbauer

The RTD failure appears to be: WARNING: failed to reach any of the inventories with the following issues: intersphinx inventory 'https://scitools.org.uk/iris/docs/latest/objects.inv' not fetchable due to <class 'requests.exceptions.HTTPError'>: 404 Client Error: Not Found for url: https://scitools.org.uk/iris/docs/latest/objects.inv which, I'm afraid, doesn't mean much to me.

I am marking this PR as ready, but please ask me for specific test cases that might be relevant and should be included.

(please definitely do not merge until I've added documentation)

Next open question: aside from zarr, few of the other backends will know what to do with fsspec's dict-like mappers. Should we prevent them from passing through? Should we attempt to distinguish between directories and files, and make fsspec file-like objects? We could just allow the backends to fail later on incorrect input.

All interested parties, please see new attempt at https://github.com/pydata/xarray/pull/4823

pint errors in xarray/tests/test_units.py::TestVariable appear unrelated

https://github.com/pydata/xarray/pull/4823 working on this. Please try and comment.

Question: should HTTP URLs be passed through unprocessed as before? I think that might be required by some of the netCDF engines, but we probably don't test this.

Docs to be added

I believe https://github.com/pydata/xarray/pull/4461 fixes this

Note that you can still use the "old" method of opening the mapper (e.g., fsspec.get_mapper) beforehand and passing that

As far as I can tell, this has only been happening in gcsfs - so my suggestion, to try to collect the set of conditions that should be considered "retryable" but currently aren't, still holds. However, it is also worthwhile discussing where else in the stack retries might be applied, which would affect multiple storage backends.

Martin has gained by implementing this PR is transferrable

I'm not sure, it's been a while now...

ping again

Please try with fsspec master.

Looks like a special case of a numpy scalar. I can catch this in fsspec - please wait.

The xarray.backends.h5netcdf_.H5NetCDFArrayWrapper seems to keep a reference to the open file, which for HTTP contains the open session. The linked PR fixes the serialization of those files, for the HTTP case.

OK, I can see a thing after all... please stand by

I don't think it's fsspec, the HTTPFileSystem and file objects are known to serialise.

However ```

distributed.protocol.serialize(dsc.surface.mean().data.dask['open_dataset-27832a1f850736a8d9a11a882ad06230surface-3b6f5b6a90c2cfa65379d3bfae22126f']) ({'serializer': 'error'}, ...) ``` (that's one of the keys I picked from the graph at random, your keys may differ) I can't say why this object is in the graph where perhaps it wasn't before, but it has a reference to a "CopyOnWriteArray", which sounds like a buffer owned by something else and probably the non-serializable part. Digging find a contained "<xarray.backends.h5netcdf_.H5NetCDFArrayWrapper at 0x17e669ad0>" which is not serializable - so maybe xarray can do something about this.

One completely unrelated failure (test_polyfit_warnings). Can I please get a final say here (@max-sixty @alexamici ?)

(failures look like something in pandas dev)

That's a lot of data!

Can you confirm that this works ok with fsspec and s3fs master?

I have reproduced it locally (also with moto). Indeed, many threads are trying to stall the event loop at once. This will take a little finesse.

Thanks for the digging, I'll look into it

File "/usr/local/lib/python3.7/asyncio/base_events.py", line 1771, in _run_once handle = self._ready.popleft()

This looks like it may be a race conditions where multiple threads are calling the event loop at once. I wonder if you could list the event loops in use and the threads (perhaps best run with base python than ipython/jupyter).

The following code, modified to the style of the s3fs test suite, works OK: ```python def test_with_xzarr(s3): da = pytest.importorskip("dask.array") xr = pytest.importorskip("xarray") name = "sample"

nana = xr.DataArray(da.zeros((1023, 1023, 3)))

s3_path = f"{test_bucket_name}/{name}"
s3store = s3.get_mapper(s3_path)

print("Storing")
nana.to_dataset().to_zarr(store=s3store, mode="w", consolidated=True, compute=True)

```

Question: to eventually get tests to pass, will need changes only just now going into zarr. Those may be released some time soon, but in the meantime is it reasonable to install from master?

Note that zarr.open* now works with fsspec URLs (in master)

The write feature for simplecache isn't released yet, of course.

It would be interesting if someone could subclass file and write locally with h5netcdf to see what kind of seeks it does. Is it popping back to some file header to update array sizes? Presumably it would need a fixed-size header to do that. Parquet and other cloud formats have the metadata at the footer exactly for this reason, so you only write once you know everything and you only ever move forward in the fie.

the behavior of zarr it appears will rely heavily on fsspec more in the future.

IF we can push on https://github.com/zarr-developers/zarr-python/pull/546 ; but here is also an opportunity to get the behaviour out of the zarr/fsspec interaction most convenient for this work.

Note that s3fs and gcsfs now expose the kwargs skip_instance_cache use_listings_cache, listings_expiry_time, and max_paths and pass them to fsspec. See https://filesystem-spec.readthedocs.io/en/latest/features.html#instance-caching and https://filesystem-spec.readthedocs.io/en/latest/features.html#listings-caching

(although the new releases for both already include the change that accessing a file, contents or metadata, does not require a directory listing, which is the right thing for zarr, where the full paths are known)

https://github.com/intake/filesystem_spec/pull/243 is where my attempt to fix this kind of thing will live.

However, writing or deleting keys should invalidate the appropriate part of the cache as it currently stands, so I don't know why the problem has arisen. If it is a cache problem, then s3.invalidate_cache() can always be called.

Note that get_mapper is implemented for all file systems, so there should be no need for any gcsfs-specific code.

On August 26, 2019 11:21:00 AM EDT, Justin Minsk notifications@github.com wrote:

Went back to gcsfs 0.0.4 and the current code still does not work. Going back more requires major changes to environment that conda cannot handle and would not be compatible with the current build of xarray. I think changing mutable maps to strings in this case has never worked with gcsfs and more than likely has not been seriously used until the append to zarr was added.

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/pydata/xarray/issues/3251#issuecomment-524903609

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

I am not sure why str should ever be called on the mapping. For sure, what it returns is not the same as before (perhaps you could go back a version and check?), but I don't know what the string would have been used for. I am on leave at the moment and unlikely to be able to get time to investigate.

Do you have any idea what is taking the extra time? s3fs ought to, in theory, treat URLs with and with the s3:// the same, but this may not have been tested with tea mapping.

I like those timings.

LGTM

Do you think there should be more explicit text of how to add consolidation to existing zarr/xarray data-sets, rather than creating them with consolidation turned on?

We may also need some text around updating consolidated data-sets, but that can maybe wait to see what kind of usage people try.

Glad to see this happening, by the way. Once in, catalogs using intake-xarray can be updated and I don't thin the code will need to change.

Will the default for both options be False for the time being?

Right now, to_zarr raises an exception in the case of irregular chunks, suggesting that the user should call .rechunk() first.

Yeah, ideally when adding a variable like ds['myvar'] = xr.DataArray(data=da.zeros(..., chunks=(..)), dims=['l', 'b', 'v']) ds.to_zarr(mapping) we should be able to apply an optimization strategy in which the zarr array is created without filling in all those unnecessary zeros. This seems doable.

On the other hand, implementing ds.myvar[slice, slice, slice] = some data ds.to_zarr(mapping) (which cannot be done currently with dask-arrays at all), in such a way that only partitions with data get updated - this seems really hard.

OK, so the way to do this in pure-zarr appears to be to simply create the appropriate zarr array and set it's dimensions attribute:

ds = xr.Dataset(coords={'b': np.arange(-4, 6, 0.005), 'l': np.arange(150, 72, -0.005), 'v': np.arange(58722.24288, -164706.4225401, -8.2446e2)}, ds.to_zarr(mapping) g = zarr.open_group(mapping) arr = g.zeros(..., shape like l, b, v) arr.attrs['_ARRAY_DIMENSIONS'] = ['l', 'b', 'v']

xr..open_zarr(mapping) now shows the new array, without having to materialize any data into it, and arr can be written to piecemeal - without the convenience of the coordinate mapping, of course.

It might be enough, in this case, to provide some helper function in zarr to create and fetch arrays that will show up as variables in xarray - this need not be specific to being used via dask. I am assuming with the work done in this PR, that there is an unambiguous way to determine if a zarr group can be interpreted as an xarray dataset, and that zarr then knows how to add things that look like variables (which generally in the zarr case don't involve writing any actual data until the parts of the array are filled in).

@jhamman , that partially solves what I mean, I can probably turn my data into dask arrays with some difficulty; but really I was hoping for something like the following: ds = xr.Dataset(coords={'b': np.arange(-4, 6, 0.005), 'l': np.arange(150, 72, -0.005), 'v': np.arange(58722.24288, -164706.4225401, -8.2446e2)}, arr = ds.create_new_zero_array(dims=['l', 'b', 'v']) arr[0:10, :, :] = 1 and expect to be able to set the values of the new variable in the same way that you can with the equivalent zarr array. I can probably get around this by setting the values with da.zeros, finding the zarr array in the dataset, and then setting its values.

Question: how would one build a zarr-xarray dataset?

With zarr you can open an array that contains no data, and use set-slice notation to fill in the values (which is what dask's store essentially does).

If I have some pre-known coordinates and bigger-than-memory data arrays, how would I go about getting the values into the zarr structure? If this can't be done directly with the xarray interface, is there a way to call zarr's open/create/zeros such that the corresponding array will appear as a variable when the same dataset is opened with xarray?

Yes, dirs exists to prevent the need to query the server for file-lists multiple times. There is an outstanding issue to move to prefix/delimited listing as with s3fs, rather than fetch the complete listing for a bucket. If all the paths are known beforehand, as might be the case for zarr, then it may be of no use at all - but actually I'm not sure then why it would have been populated.

I am puzzled that serializing the mapping is pulling the data. GCSMap does not have get/set_state, but the only attributes are the GCSFileSystem and path. Perhaps the __getitem__ gets called? As for the GCSFileSystem, it stores the token with a renewable token, which lives indefinitely, and the refresh API is called upon deserialization. There should probably be a check in _call to ensure that the token hasn't expired.

This is, of course, by design :) I imagine there is much that could be done to optimise performance, but for fewer, larger chunks, it should be pretty good.

hdfs3 also has a MutableMapping for HDFS. I did not succeed in getting one into azure-datalake-store, but it would not be hard to make. In this way, zarr can become a pretty general array cloud storage mechanism.

I have not done anything, I'm afraid, since posting my commit, the content of which is just an example of how you might pass parameters down to zarr, and a test-case which shows that the basic data is round-tripping properly, but actually the dataset does not come back with the same structure as it started off. We can loop back and decide where to go from here.

@shoyer , is https://github.com/martindurant/xarray/commit/6c1fb6b76ebba862a1c5831210ce026160da0065 a reasonable start ?

@rabernat , is there anything I can do to help push this along?

A further rather big advantage in zarr that I'm not aware of in cdf/hdf (I may be wrong) is not just null values, but not having a given block be written to disc at all if it only contains null data. This probably meshes perfectly well with most user's understanding of missing data/fill value.

Worth pointing out here, that the zarr filter-set is extensible (I suppose hdf5 is too, but I don't think this is ever done in practice), but I don't think it makes any particular claims to performance.

I think both of the options above are reasonable, and there is no particular reason to exclude either: a zarr variable could look to xarray like floats but actually be stored as ints (i.e., arguments are passed to zarr), or it could look like ints which xarray expects to inflate to floats (i.e., stored as an attribute). I mean, if a user stores a float variable, but includes kwargs to zarr for scale/filter (or any other filter arguments), we should make no attempt to interrupt that.

The only question is, if the user wishes to apply scale/offset in xarray, which is their most likely intention? I would guess the latter, compute in xarray and use attributes, since xarray users probably don't know about zarr and its filters.

@rabernat : on actually looking through your code :) Happy to see you doing exactly as I felt I was not knowledgeable to do and poke xarray's guts. If I can help in any way, please let me know, although I don't have a lot of spare hours right now.

Sorry that I let this slide - there was not a huge upswell of interest around what I had done, and I was not ready to dive into xarray internals. Could you comment more on the difference between your approach and mine? Is the aim to reduce the number of metadata files hanging around? zarr has made an effort with the groups interface to parallel netCDF, which is, after all, what xarray essentially expects of all its data sources.

As in this comment I have come to the realisation that although nice to/from zarr methods can be made relatively easily, they will not get traction unless they can be put within a class that mimics the existing xarray infrastructure, i.e., the user would never know, except that magically they have extra encoding/compression options, the file-path can be an S3 URL (say), and dask parallel computation suddenly works on a cluster and/or with out-of-core processing. That would raise some eyebrows!

@alimanfoo , do you think this work would make more sense as part of zarr rather than as part of xarray?

True, xarray_to_zarr is unchanged from before. The dataset functions could supercede, since a single xarray is just a special case of a dataset; or we could decide that for the special case it is worth having short-cut functions. I was worried about the number of metadata files being created, since on a remote system like S3, there is a large overhead to reading many small files.

@alimanfoo , in the new dataset save function, I do exactly as you suggest, with everything getting put as a dict into the main zarr group attributes, with special attribute names "attrs" for the data-set root, "coords" for the set of coordinate objects and "variables" for the set of variables objects (all of these have their own attributes in xarray).

I have developed my example a little to sidestep subclassing you suggest, which seemed tricky to implement.

Please see https://gist.github.com/martindurant/06a1e98c91f0033c4649a48a2f943390 (dataset_to/from_zarr functions)

I can use the zarr groups structure to mirror at least typical use of xarrays: variables, coordinates and sets of attributes on each. I have tested this with s3 too, stealing a little code from dask to show the idea.

3: a json-like representation such as used by the hidden .xarray item would also do.