Skip to content

Conversation

@pelson
Copy link
Member

@pelson pelson commented Jun 25, 2017

I've done this with very little understanding of the original rationale for integrating iris_grib back into iris, so I may have missed a key detail.

@pelson pelson requested review from bjlittle and pp-mo June 25, 2017 06:17
@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 83.786% when pulling 4d0efeb on dask_iris into 1dd3aa6 on master.

@pelson
Copy link
Member Author

pelson commented Jun 25, 2017

Note: I accidentally pushed this branch to scitools/iris-grib, rather than my own fork. Apologies for that.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 83.786% when pulling 6c9fb54 on dask_iris into 1dd3aa6 on master.

pelson referenced this pull request in SciTools/iris Jun 25, 2017
Fix translation generator tool to use internal grib module.
@pelson
Copy link
Member Author

pelson commented Jun 25, 2017

For the record, the pertinent commit in Iris that brought iris_grib back in was: SciTools/iris@e45acf7#diff-3967991fd23406c94b677376dc007fd8

__slots__ = ('shape', 'dtype', 'fill_value', 'recreate_raw')
__slots__ = ('shape', 'dtype', 'recreate_raw')

def __init__(self, shape, dtype, fill_value, recreate_raw):
Copy link
Member Author

Choose a reason for hiding this comment

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

This looks like an oversight in iris.

@lbdreyer
Copy link
Member

I've done this with very little understanding of the original rationale for integrating iris_grib back into iris

You have been shown this before but in case you forgot, there was an attempt to open the discussion about the future of iris_grib on the goole groups.

I don't understand the purpose of this PR. As far as I'm concerned iris_grib is dead

@pelson
Copy link
Member Author

pelson commented Jun 28, 2017

Thanks @lbdreyer. The link was helpful - I've written something there, and suggested we get together to discuss iris' direction in a slightly higher-bandwidth form. (doodle poll for date/time: https://doodle.com/poll/yaxm4pqfkus96sm2)

Suffice to say, I personally do not feel that the subject has been considered sufficiently to call iris-grib a dodo just yet...

@pelson
Copy link
Member Author

pelson commented Aug 16, 2017

The thread was: https://groups.google.com/forum/#!topic/scitools-iris-dev/lFw2_CnaEHU
The direction is for SciTools/iris-grib to be the canonical GRIB loading/saving capability for Iris.

In light of this, could this please get a review? @SciTools/iris-grib-devs

@pp-mo pp-mo merged commit 3c0c286 into master Oct 17, 2017
@pp-mo
Copy link
Member

pp-mo commented Oct 17, 2017

I'm happy that this is equivalent to code in Iris at "version 2.0a0", i.e. SciTools/iris@7ec22fa.
There is a small code difference between the load_convert.ellipsoid routines, between use of if and elif :
( compare : https://github.com/SciTools/iris-grib/blob/6c9fb54b01eeac9ebeeec4a5337223159b31efae and /iris_grib/_load_convert.py#L387 )
I'm satisfied that that, and other differences, are immaterial.

Note that this version only works with current iris master branch, at the "2.0a0" version (which is not a tag, to avoid problems with conda-forge : see above SHA for reference).
This "2.0a0 alpha release" version contains the 'dask nanmask solution' code : It is ephemeral + will not be made available via a tag or release.
So, we need to fix again soon wrt the imminent mergeback of the feature branch https://github.com/SciTools/iris/tree/dask_mask_array

@pelson pelson deleted the dask_iris branch October 19, 2017 11:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants