Python3 grib fixes #1774

QuLogic · 2015-09-07T22:07:15Z

A couple of small fixes for GRIB parsing. Probably a more permanent fix should be put in GRIB itself, but I was lazy and some stuff was not making sense so I wasn't sure of the best fix anyway.

GRIB does not handle integral types correctly on Python 3.

For some reason, the file seems to read in 4K chunks, even if I disable buffering. So simply re-seek to the location of the end of the message after reading one.

rhattersley · 2015-09-09T09:30:04Z

These both seem like genuine bugs. Have you raised them upstream?

rhattersley · 2015-09-09T09:31:55Z

Do you know what the performance impact is of the extra seeks?

ajdawson · 2015-09-09T09:34:51Z

I don't think ECMWF support gribapi on Python 3 anyway do they?

Are you using the SciTools conda package for gribapi on Python 3? It looks like that applies a patch for Python 3 compatibility, so could that be a potential location of Python 3 problems too?

QuLogic · 2015-09-09T21:23:06Z

I'm aware of the patch; I wrote it. 😉 I put the changes in here because it's easier to iterate than rebuilding the conda package, and I'd like some advice (though I probably should have explained that earlier.)

For grib_set_array, the GRIB code checks for instances of int, which fails on Python 3 because NumPy arrays are no longer subclasses of int but of numbers.Integral. Shouldn't be too troublesome to fix.

For the seeking though, I really have no idea what's causing it. Whenever GRIB reads something, no matter the size, the file pointer always advances by 4K. I thought maybe Python 3's file had some buffering, but I can't find any way to disable it (or what I tried didn't work.) Admittedly, I'd say the file->FILE* conversion for Python 3 GRIB is a bit of a hack, but so long as the API is constrained to those types, it'll have to do.

ajdawson · 2015-09-10T07:44:10Z

My concern is that this fix will go in for compatibility against a patched gribapi version, and will remain in our code base even if/when Python 3 is actually supported by gribapi.

QuLogic · 2015-09-10T08:16:19Z

I can update the patched GRIB for the first commit easily (though it doesn't really hurt to be more explicit there.) However, I was really hoping someone might be able to come up with a better workaround or fix for the second commit's issue.

rhattersley · 2015-09-10T08:20:02Z

Could we just leave GRIB unsupported in Python 3 for now? (e.g. Skip the relevant tests, perhaps add a warning when using it?)

ajdawson · 2015-09-10T08:24:15Z

The first commit I have no issue with, it is more the second one. I'm tempted to say that we should leave GRIB support out of the Python 3 version initially until this has been thought through some more and we come up with a good solution (although it pains me to say that as I mostly work with GRIB data...).

QuLogic · 2015-09-11T02:21:23Z

We could leave GRIB unsupported, but it would require added the relevant skips, so I'm against it on account of laziness 😉.

To go forward though, the best method is either for GRIB to provide an open/close function to get the FILE* from C code (and stop allowing Python's file), or to add some wrapper for I/O for C->Python APIs.

rhattersley · 2015-09-18T16:05:38Z

NB. I'm experimenting with making GRIB an optional dependency - it's something I've wanted to do for a long time.

ocefpaf · 2015-09-18T16:16:14Z

NB. I'm experimenting with making GRIB an optional dependency - it's something I've wanted to do for a long time.

Huge 👍

rhattersley · 2015-09-21T11:49:45Z

NB. I'm experimenting with making GRIB an optional dependency - it's something I've wanted to do for a long time.

See #1789.

rhattersley · 2015-09-22T14:35:15Z

And it's done! Thanks to #1789 GRIB is now officially optional. 😀

rhattersley · 2015-09-22T15:27:30Z

Chatting with @marqh offline he pointed out that the current gribapi from ECMWF is effectively deprecated now they are putting their effort into ecCodes.

That adds weight to the argument that we drop GRIB support from Python 3. We can add it later via ecCodes.

QuLogic · 2015-09-22T21:37:04Z

If I read their description correctly, the ecCodes API should be the same as GRIB-API, so it might have issues with Python 3 as well. If anyone's got any sway there, you should see if they're testing that out too...

marqh · 2015-09-23T08:38:00Z

I'm aiming to get some time to get Iris to move over to ecCodes.

If claims are to be believed, it will be a simple piece of work to deliver once I get started; we shall see.

I will use this work to investigate with ECMWF the limitations on Python version and raise issues with them. Hopefully we'll get things moving there.

If we can hit v1.9 by making GRIB py3 optional (or already have done) then that's a fine solution from my point of view

rhattersley · 2015-09-23T08:45:57Z

So... what do we want to do? Close this PR and remove the Py3 builds from conda (and conda-recipes-scitools)?

marqh · 2015-09-24T07:43:34Z

So... what do we want to do? Close this PR and remove the Py3 builds from conda (and conda-recipes-scitools)?

that sounds like a sensible approach to me

ajdawson · 2015-09-24T14:39:33Z

So... what do we want to do? Close this PR and remove the Py3 builds from conda (and conda-recipes-scitools)?

I think so, although we might want to keep the first commit from this PR to save it from having to be rediscovered at a later time when Python 3 support is available for gribapi.

We also need to add a note to the what's new entry for Python 3 support stating that GRIB is not supported in Iris on Python 3.

marqh · 2015-10-12T14:48:36Z

I've pushed up a PR on Iris to add a what's new entry, indicating that GRIB is unsupported for Python 3

Is that enough to close this PR, or to take it out of the 1.9 milestone?
what's the better approach?

I think so, although we might want to keep the first commit from this PR to save it from having to be rediscovered at a later time when Python 3 support is available for gribapi.

I think we can push this information into an issue, would that be all right?

Do we need to pull things out of conda-recipes? I thought we could leave them in there; GRIB is an optional dependency now, so I'd have thought this would be all right

cheers
mark

pelson · 2015-10-12T14:59:51Z

lib/iris/fileformats/grib/_save_rules.py

-                           np.array(np.round(lon_values), dtype=np.int64))
-    gribapi.grib_set_array(grib, 'latitudes',
-                           np.array(np.round(lat_values), dtype=np.int64))
+    gribapi.grib_set_long_array(grib, 'longitudes',


Does this need to be incorporated into Iris?

Does this need to be incorporated into Iris?

my belief: not until Python3 is adopted

marqh · 2015-10-14T09:42:29Z

#1806 is now merged

please may I suggest that we close this PR and move details to #1807 for ongoing management

This will then be out of scope for the v1.9 milestone

many thanks
marqh

marqh · 2015-10-14T09:42:49Z

@QuLogic does this sound all right to you?

QuLogic · 2015-10-14T19:19:34Z

Sounds good to me.

QuLogic added 2 commits September 7, 2015 18:03

py3k: Be explicit about GRIB array type.

4fcd55a

GRIB does not handle integral types correctly on Python 3.

py3k: Re-set GRIB file loader after every message.

e6b25f7

For some reason, the file seems to read in 4K chunks, even if I disable buffering. So simply re-seek to the location of the end of the message after reading one.

QuLogic mentioned this pull request Sep 7, 2015

Python 3 support #1658

Closed

14 tasks

QuLogic added the python3 label Sep 12, 2015

QuLogic modified the milestone: v1.9 Sep 12, 2015

marqh mentioned this pull request Oct 12, 2015

Grib Py3 whats new #1806

Merged

pelson reviewed Oct 12, 2015
View reviewed changes

marqh mentioned this pull request Oct 14, 2015

Python3 GRIB provision #1807

Closed

QuLogic closed this Oct 14, 2015

pp-mo mentioned this pull request Nov 5, 2015

Py3k consistent unicode handling #1782

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python3 grib fixes #1774

Python3 grib fixes #1774

QuLogic commented Sep 7, 2015

rhattersley commented Sep 9, 2015

rhattersley commented Sep 9, 2015

ajdawson commented Sep 9, 2015

QuLogic commented Sep 9, 2015

ajdawson commented Sep 10, 2015

QuLogic commented Sep 10, 2015

rhattersley commented Sep 10, 2015

ajdawson commented Sep 10, 2015

QuLogic commented Sep 11, 2015

rhattersley commented Sep 18, 2015

ocefpaf commented Sep 18, 2015

rhattersley commented Sep 21, 2015

rhattersley commented Sep 22, 2015

rhattersley commented Sep 22, 2015

QuLogic commented Sep 22, 2015

marqh commented Sep 23, 2015

rhattersley commented Sep 23, 2015

marqh commented Sep 24, 2015

ajdawson commented Sep 24, 2015

marqh commented Oct 12, 2015

pelson Oct 12, 2015

marqh Oct 12, 2015

marqh commented Oct 14, 2015

marqh commented Oct 14, 2015

QuLogic commented Oct 14, 2015

Python3 grib fixes #1774

Python3 grib fixes #1774

Conversation

QuLogic commented Sep 7, 2015

rhattersley commented Sep 9, 2015

rhattersley commented Sep 9, 2015

ajdawson commented Sep 9, 2015

QuLogic commented Sep 9, 2015

ajdawson commented Sep 10, 2015

QuLogic commented Sep 10, 2015

rhattersley commented Sep 10, 2015

ajdawson commented Sep 10, 2015

QuLogic commented Sep 11, 2015

rhattersley commented Sep 18, 2015

ocefpaf commented Sep 18, 2015

rhattersley commented Sep 21, 2015

rhattersley commented Sep 22, 2015

rhattersley commented Sep 22, 2015

QuLogic commented Sep 22, 2015

marqh commented Sep 23, 2015

rhattersley commented Sep 23, 2015

marqh commented Sep 24, 2015

ajdawson commented Sep 24, 2015

marqh commented Oct 12, 2015

pelson Oct 12, 2015

Choose a reason for hiding this comment

marqh Oct 12, 2015

Choose a reason for hiding this comment

marqh commented Oct 14, 2015

marqh commented Oct 14, 2015

QuLogic commented Oct 14, 2015