g1_level1 handle surface level type

[munslowa]

grib 1 file has low cloud cover grib message.
indicatorOfParameter = 186;
indicatorOfParameter = 186;

Surface (of the Earth, which includes sea surface) (grib1/3.table)

indicatorOfTypeOfLevel = 1;
level = 0;

This pull request is to update the rules to handle this type of level.

[bblay]

PR submitted to this branch to fix a typo.

[pelson]

This is the typo @bblay is talking about. I'm not sure whether this works or not (it used to), but I agree we should use the equality operator here.

[pelson]

Thanks for submitting @munslowa.

[bblay]

I believe @pp-mo is taking ownership of this now?

[pp-mo]

@bblay I believe @pp-mo is taking ownership of this now?

Yes, this is for a specific purpose. I'm keeping it on hold for now, as I'm expecting it to require some other similar changes, which (if simple) it may be better to combine within one PR.

[pp-mo]

In my subsequent work on a similar issue, I've found that this code has another misunderstanding ...
It seems that the gribapi defaults have changed so that default access to this parameter (as in "message.attribute") now returns a string instead of a number as previously. So instead of numbers 1 and 100, we now get 'sfc' and 'pl' (i.e. "surface" and "pressure level").
Also note, the alternative parameters levelType and indicatorOfTypeOfLevel aren't different (as you might expect), but behave exactly the same (and they both now return strings in this way).

So, I believe this whole section should now read :

IF
grib.edition == 1
grib.levelType == 'pl'
THEN
CoordAndDims(DimCoord(points=grib.level, long_name="pressure", units="hPa"))

IF
grib.edition == 1
grib.levelType = 'sfc'
THEN
CoordAndDims(DimCoord(points=0, long_name="height", units="m"))

[bblay]

Hmm. Changing data types (and contents) of "computed keys", such as levelType, has been problem for many years. However, I didn't expect "coded keys", such as indicatorOfTypeOfLevel, to suffer from this as they should reflect the grib spec, which states the data type. Because we've been using a computed key here for some time I can't be sure if this is a recent change or if it has always been decoded as a string (https://software.ecmwf.int/issues/browse/SUP-484).

I suggest we remove this vulnerability. We can force GribWrapper to give us the coded integer value for this key in GribWrapper.__getattr__, as we currently do for typeOfFirstFixedSurface.

[pp-mo]

@bblay I believe @pp-mo is taking ownership of this now?
@pp-mo Yes, this is for a specific purpose. I'm keeping it on hold for now, as I'm expecting it to require some other similar changes, which (if simple) it may be better to combine within one PR.

I have a new branch with extra Grib i/o functionality that includes this -- but it's not ready to submit.

So we can still merge this separately if wanted.

[bblay]

I've taken the liberty of closing this PR as it's now covered by #482.

[bblay]

For completeness, I should point out that the solution in #482 addresses the problem that spawned this PR and does not provide a generic 'surface level' loading capability.