Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get working with iris-grib-with-eccodes; Python 3 included. #2887

Merged
merged 3 commits into from
Oct 30, 2017

Conversation

pp-mo
Copy link
Member

@pp-mo pp-mo commented Oct 27, 2017

This won't work until a new iris-grib>=0.12 is available on conda-forge
I'm assuming that will have masked dask support AND eccodes

There are changes here specific to the eccodes transition, ported across from #2607

@pp-mo pp-mo mentioned this pull request Oct 27, 2017
@pp-mo pp-mo changed the title Add iris-grib as a testing dependency; Python 3 included. Get working with iris-grib-with-eccodes; Python 3 included. Oct 27, 2017
Copy link
Member

@pelson pelson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pp-mo. This looks good to me. 👍

@@ -36,4 +35,4 @@ python-stratify
pyugrid

# Iris extensions (i.e. key tools that depend on Iris)
# iris_grib
iris_grib>=0.12
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I thought this was commented out for a reason!? iris extensions shouldn't be installed by iris because they themselves depend on Iris.

@@ -330,7 +330,7 @@ def save(source, target, saver=None, **kwargs):
* netCDF - the Unidata network Common Data Format:
* see :func:`iris.fileformats.netcdf.save`
* GRIB2 - the WMO GRIdded Binary data format:
* see :func:`iris.fileformats.grib.save_grib2`.
* see :func:`iris_grib.save_grib2`.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

'SKIPPING test for this cube, as save/load will '
'not currently work.')
warnings.warn(msg.format(i, name_cube.name()))
continue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(I know it 's not your change but... ) We have more sophisticated means for skipping tests...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this isn't skipping a test, it's skipping one of the cubes tested within a single testcase.
As the set of cases is defined by content from a file, rather than the test code, I can't really see a better way ...

@@ -34,6 +34,7 @@

if tests.GRIB_AVAILABLE:
import gribapi
from iris_grib._load_convert import _MDI as MDI
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a private API, we obviously we shouldn't be relying on this, but happy enough for now.

@pelson
Copy link
Member

pelson commented Oct 28, 2017

FWIW: Technically, we could not merge this in 2.0.0 and still have iris' GRIB capability using eccodes the moment we update iris-grib. I consider that to be a healthy sign of separation.

As it stands, there are some good doc improvements in here that make sense to have in v2.0.0

@pelson pelson force-pushed the post_iris2_grib_fixes branch from 4833ba3 to 94ae63b Compare October 28, 2017 05:43
@pelson pelson merged commit 9aad938 into SciTools:master Oct 30, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants