Added PNC backend to xarray #1905
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,3 +25,4 @@ dependencies: | |
| - pytest-cov | ||
| - pydap | ||
| - lxml | ||
| - PseudoNetCDF | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,120 @@ | ||
| from __future__ import absolute_import | ||
| from __future__ import division | ||
| from __future__ import print_function | ||
|
|
||
| import functools | ||
|
|
||
| import numpy as np | ||
|
|
||
| from .. import Variable | ||
| from ..core.pycompat import OrderedDict | ||
| from ..core.utils import (FrozenOrderedDict, Frozen) | ||
| from ..core import indexing | ||
|
|
||
| from .common import AbstractDataStore, DataStorePickleMixin, BackendArray | ||
|
|
||
|
|
||
| class PncArrayWrapper(BackendArray): | ||
|
|
||
| def __init__(self, variable_name, datastore): | ||
| self.datastore = datastore | ||
| self.variable_name = variable_name | ||
| array = self.get_array() | ||
| self.shape = array.shape | ||
| self.dtype = np.dtype(array.dtype) | ||
|
|
||
| def get_array(self): | ||
| self.datastore.assert_open() | ||
| return self.datastore.ds.variables[self.variable_name] | ||
|
|
||
| def __getitem__(self, key): | ||
| key, np_inds = indexing.decompose_indexer( | ||
| key, self.shape, indexing.IndexingSupport.OUTER_1VECTOR) | ||
|
|
||
| with self.datastore.ensure_open(autoclose=True): | ||
| array = self.get_array()[key.tuple] # index backend array | ||
|
|
||
| if len(np_inds.tuple) > 0: | ||
| # index the loaded np.ndarray | ||
| array = indexing.NumpyIndexingAdapter(array)[np_inds] | ||
| return array | ||
|
|
||
|
|
||
| _genericncf = ('Dataset', 'netcdf', 'ncf', 'nc') | ||
|
|
||
|
|
||
| class _notnetcdf: | ||
| def __eq__(self, lhs): | ||
| return lhs not in _genericncf | ||
|
|
||
|
|
||
| class PseudoNetCDFDataStore(AbstractDataStore, DataStorePickleMixin): | ||
| """Store for accessing datasets via PseudoNetCDF | ||
| """ | ||
| @classmethod | ||
| def open(cls, filename, format=None, writer=None, | ||
| autoclose=False, **format_kwds): | ||
| from PseudoNetCDF._getreader import getreader, getreaderdict | ||
|
There was a problem hiding this comment. Importing from private APIs (with the underscore prefix) makes me nervous. In general we try to avoid doing this because private APIs can change at any time, though of course that isn't true here because you're the PNC maintainer. I'm also nervous about including constants like So maybe we should stick with the standard open method that you were using earlier. In particular, if we change the default to There was a problem hiding this comment. I think it will be confusing for users to have an indirect use of I agree completely about private API. The standard pncopen, however, does not currently support the ability to enable a subset of readers. I will add this a feature request to PseudoNetCDF, and make a subsequent update there. As soon as I update pncopen, I would make the update here. The The alternative is to update PNC, update the conda installation, and update this code again. There was a problem hiding this comment. Are you thinking of someone explicitly calling Another way to make all of this safe (and which wouldn't require disabling There was a problem hiding this comment. Imagine that you have a netcdf file. If you open it with PNC, it uses netCDF4 as the library to open it. In xarray, however, you might get a different processing. If you write it out, it might have different properties. This seems undesirable at best. That is why I prefer removing the netCDF functionality. In PNC, the goal of using netCDF4 is to make sure all pnc functionality is available for any type of file. In PNC, some plotting and processing tools are available (similar to xarray). I think plotting and processing in xarray is often better than PNC. The goal of adding PNC to xarray is to bring xarray formats that PNC can read. Since xarray already has mechanisms for supporting netCDF, I think bringing this functionality is unnecessary. There was a problem hiding this comment. My general feeling is that we should make it easy to do the right thing, but not put up artificial barriers to stop expert users. I think this is broadly consistent with most Python projects, including the Python language itself. Even if it's strange, somebody might have reasons for trying PNC/netCDF4 together, and as long as it will not give wrong results there is no reason to disable it. That said, my real objection here are all the private imports from PNC, and the four different aliases we use to check for netCDF files. If you want to put a public API version of this in PNC before merging this PR, that would be OK with me. There was a problem hiding this comment. Okay. I removed the type checking and private imports. I updated the testing. I get one failed that I cannot identify as related to PNC: I'm going to run it through the build to see if it is just my setup. There was a problem hiding this comment. It didn't fail on the automated test, so all passing [x] and no private imports [x]. |
||
| readerdict = getreaderdict() | ||
|
There was a problem hiding this comment. F841 local variable 'readerdict' is assigned to but never used There was a problem hiding this comment. F841 local variable 'readerdict' is assigned to but never used |
||
| reader = getreader(filename, format=format, **format_kwds) | ||
| _genreaders = tuple([readerdict[rn] for rn in _genericncf]) | ||
| if isinstance(reader, _genreaders): | ||
| raise ValueError(('In xarray, PseudoNetCDF should not be used ' + | ||
|
There was a problem hiding this comment. I think we can avoid this check/error message if we default to There was a problem hiding this comment. see previous comment. |
||
| 'to read netcdf files with unknown metadata. ' + | ||
| 'Instead, use netcdf4. If this is a known ' + | ||
| 'format, specify it using the format keyword ' + | ||
| '(or backend_kwargs={\'format\': <name>} from ' + | ||
| 'open_dataset).')) | ||
|
|
||
| opener = functools.partial(reader, filename, **format_kwds) | ||
| ds = opener() | ||
| mode = format_kwds.get('mode', 'r') | ||
| return cls(ds, mode=mode, writer=writer, opener=opener, | ||
| autoclose=autoclose) | ||
|
|
||
| def __init__(self, pnc_dataset, mode='r', writer=None, opener=None, | ||
| autoclose=False): | ||
|
|
||
| if autoclose and opener is None: | ||
| raise ValueError('autoclose requires an opener') | ||
|
|
||
| self._ds = pnc_dataset | ||
| self._autoclose = autoclose | ||
| self._isopen = True | ||
| self._opener = opener | ||
| self._mode = mode | ||
| super(PseudoNetCDFDataStore, self).__init__() | ||
|
|
||
| def open_store_variable(self, name, var): | ||
| with self.ensure_open(autoclose=False): | ||
| data = indexing.LazilyOuterIndexedArray( | ||
| PncArrayWrapper(name, self) | ||
| ) | ||
| attrs = OrderedDict((k, getattr(var, k)) for k in var.ncattrs()) | ||
| return Variable(var.dimensions, data, attrs) | ||
|
|
||
| def get_variables(self): | ||
| with self.ensure_open(autoclose=False): | ||
| return FrozenOrderedDict((k, self.open_store_variable(k, v)) | ||
| for k, v in self.ds.variables.items()) | ||
|
|
||
| def get_attrs(self): | ||
| with self.ensure_open(autoclose=True): | ||
| return Frozen(dict([(k, getattr(self.ds, k)) | ||
| for k in self.ds.ncattrs()])) | ||
|
|
||
| def get_dimensions(self): | ||
| with self.ensure_open(autoclose=True): | ||
| return Frozen(self.ds.dimensions) | ||
|
|
||
| def get_encoding(self): | ||
| encoding = {} | ||
| encoding['unlimited_dims'] = set( | ||
| [k for k in self.ds.dimensions | ||
| if self.ds.dimensions[k].isunlimited()]) | ||
| return encoding | ||
|
|
||
| def close(self): | ||
| if self._isopen: | ||
| self.ds.close() | ||
| self._isopen = False | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you move this after PyNIO and before pandas? I think that would make a little more logical sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure.