issue_comments

@shoyer I added the TestCfGrib class with basic read tests, a test GRIB file and ecCodes / cfgrib to requirements-py36.yml.

Questions: 1. ~~I failed to reach 100% coverage becasue I didn't find the way to test CfGribDataStore.get_dimensions. Any hint?~~ Done. 2. Can /should I test dask and dask.distributed support in the unit tests? How? Any pointers?

It looks like we are very close, what do you think? Shall I move to the documentation?

Congratulations!

Thanks @alexamici and ECMWF!

Hello @alexamici! Thanks for updating the PR.

• There are no PEP8 issues in the file xarray/backends/__init__.py !

• There are no PEP8 issues in the file xarray/backends/api.py !

• There are no PEP8 issues in the file xarray/backends/cfgrib_.py !

• There are no PEP8 issues in the file xarray/tests/__init__.py !

• There are no PEP8 issues in the file xarray/tests/test_backends.py !

• There are no PEP8 issues in the file xarray/tests/test_distributed.py !

Comment last updated on October 15, 2018 at 13:59 Hours UTC

@shoyer I added a test for dask.distributed and it passes but please check that it is meaningful, as I'm not completely sure what to test.

Do you usually keep the history of PRs as it is or do you prefer me to rebase?

We squash commits upon merging, so please leave things as is :)

@shoyer I'm ready to integrate more feedback (especially on the documentation), but I removed the WIP: prefix as I'd consider the PR good to go as soon as you like it.

Do you usually keep the history of PRs as it is or do you prefer me to rebase?

Tests added and passing with 100% coverage. Added minimal documentation.

BTW, I did a 0.9.0 beta release of cfgrib and I plan to give the public announcement tomorrow. :)

Tests should be fixed if you merge in master now.

@alexamici oops, we accidentally disabled most of our backend unit tests -- see https://github.com/pydata/xarray/pull/2479 for the fix.

@shoyer thank you very much for the guidance, it is very appreciated!

I stared working on the tests, but I've been blocked immediately by something that looks trivial. I cannot get the test class PyNioTest to run: $ python setup.py test --addopts -v | grep Test ... xarray/tests/test_backends.py::TestCommon::test_robust_getitem PASSED [ 2%] xarray/tests/test_backends.py::TestRasterio::test_serialization SKIPPED [ 3%] xarray/tests/test_backends.py::TestRasterio::test_utm SKIPPED [ 4%] xarray/tests/test_backends.py::TestRasterio::test_non_rectilinear SKIPPED [ 4%] xarray/tests/test_backends.py::TestRasterio::test_platecarree SKIPPED [ 4%] xarray/tests/test_backends.py::TestRasterio::test_notransform SKIPPED [ 4%] xarray/tests/test_backends.py::TestRasterio::test_indexing SKIPPED [ 4%] xarray/tests/test_backends.py::TestRasterio::test_caching SKIPPED [ 4%] xarray/tests/test_backends.py::TestRasterio::test_chunks SKIPPED [ 4%] xarray/tests/test_backends.py::TestRasterio::test_pickle_rasterio SKIPPED [ 4%] xarray/tests/test_backends.py::TestRasterio::test_ENVI_tags SKIPPED [ 4%] xarray/tests/test_backends.py::TestRasterio::test_no_mftime SKIPPED [ 4%] xarray/tests/test_backends.py::TestRasterio::test_http_url SKIPPED [ 4%] xarray/tests/test_backends.py::TestEncodingInvalid::test_extract_nc4_variable_encoding PASSED [ 4%] xarray/tests/test_backends.py::TestEncodingInvalid::test_extract_h5nc_encoding PASSED [ 4%] xarray/tests/test_backends.py::TestValidateAttrs::test_validating_attrs PASSED [ 4%] xarray/tests/test_backends.py::TestDataArrayToNetCDF::test_dataarray_to_netcdf_no_name PASSED [ 4%] xarray/tests/test_backends.py::TestDataArrayToNetCDF::test_dataarray_to_netcdf_with_name PASSED [ 4%] xarray/tests/test_backends.py::TestDataArrayToNetCDF::test_dataarray_to_netcdf_coord_name_clash PASSED [ 4%] xarray/tests/test_backends.py::TestDataArrayToNetCDF::test_open_dataarray_options PASSED [ 4%] xarray/tests/test_backends.py::TestDataArrayToNetCDF::test_dataarray_to_netcdf_return_bytes PASSED [ 4%] xarray/tests/test_backends.py::TestDataArrayToNetCDF::test_dataarray_to_netcdf_no_name_pathlib PASSED [ 4%] ... How do you make pytest run test classes that do not start with Test?

The v0.11 release still need more work (to complete deprecations), so it's at least a week or two out (probably more). I suspect we could get this PR ready together in time.

On Tue, Oct 9, 2018 at 3:59 PM Alessandro Amici notifications@github.com wrote:

@shoyer https://github.com/shoyer BTW what timeframe do you expect for the v0.11 release? And would you consider merging this Pull Request before the release, assuming that we do a cfgrib release with read support declared beta?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/pydata/xarray/pull/2476#issuecomment-428381697, or mute the thread https://github.com/notifications/unsubscribe-auth/ABKS1nCJOGlFSIEtL28sQ80GZqhzJ_Q2ks5ujSpVgaJpZM4XOXA8 .

@shoyer BTW what timeframe do you expect for the v0.11 release? And would you consider merging this Pull Request before the release, assuming that we do a cfgrib release with read support declared beta?

The cfgrib_.py backend now uses the new CachingFileManager interface and I added a global process lock, just-in-case. The code is very simple and I mimicked the PyNIO backend so I expect it to work correctly with dask, but I'm not sure how to test for it.

Furthermore I expect dask performance to be abysmal until I implement ecmwf/cfgrib#20.

The appropriate lock to use depends on cfgrid. Is the library thread-safe? If not, it's probably best to use a global (per process) lock.

pynio is probably the simplest example of how to write a new backend: https://github.com/pydata/xarray/blob/master/xarray/backends/pynio_.py

The main difference is that you should make use of CachingFileManager to add support for dask and opening many files at once. File managers make it possible to pickle a datastore, which is what we need to make dask-distirbuted work.

@shoyer at long last! :)

I quickly sync'ed with the new backend API. I did some light testing.

Note that I didn't test with dask at all and I'm not using the passed lock. Pointers to how to properly use the new backend interface are welcome.

@alexamici great to see this!

I'm about to merge a refactor of xarray's backends for v0.11 in https://github.com/pydata/xarray/pull/2261. You'll need to refactor a little bit to accommodate this, but hopefully that should be straightforward. The new interface should be a little easier to use, especially for handling many files or dask-distributed support.