search

metno / pysurfex

Python API to SURFEX. Tool to create offline SURFEX forcing, SURFEX files, running SURFEX and plot results.
10 stars 16 forks source link

Update bufr.py for carra2 for local observations #35

Closed PerDahlgren closed 9 months ago

PerDahlgren commented 9 months ago

Added features in bufr.py for carra2 to be able to read local observations

trygveasp commented 9 months ago

Can you try to add only your changes? You have removed some of the last developments I think (sigmao etc)

trygveasp commented 9 months ago

The unit tests fail because we now require stationOrSiteName in the bufr file. Is it ok to have such a requirement @PerDahlgren ? Will this key always be existing?

PerDahlgren commented 9 months ago

stationOrSiteName will not always be existing, only in some bufr templates. I am a bit confused because this worked when I ran harmonie, with bufr input that was a mix of different bufr-templates, some with stationOrSiteName and some without it.

tir. 16. jan. 2024 kl. 10:14 skrev Trygve Aspelien @.***

:

The unit tests fail because we now require stationOrSiteName in the bufr file. Is it ok to have such a requirement @PerDahlgren https://github.com/PerDahlgren ? Will this key always be existing?

— Reply to this email directly, view it on GitHub https://github.com/metno/pysurfex/pull/35#issuecomment-1893341930, or unsubscribe https://github.com/notifications/unsubscribe-auth/AWUSAIVUIDOMLTBU5PDO4WTYOZAHRAVCNFSM6AAAAABBX27BRKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOJTGM2DCOJTGA . You are receiving this because you were mentioned.Message ID: @.***>

PerDahlgren commented 9 months ago

The fail, in the test, seems to come from eccodes.codes_get(bufr, key). In Bufr2json (in Harmonie) this does not seem give a crash, it just continues with the next key. I see that different eccodes versions are used: 2.20.0 (used by the test), 2.23.0 in my Harmonie experiment Can that be the reason?

joewkr commented 9 months ago

@PerDahlgren, this crash has nothing to do with eccodes' version since when running the testsuite real reading functions of eccodes are replaced by stubs which return values from a test BUFR file implemented as a python dictionary (see conftest.py#L425-L447). Since in your PR you add a new stationOrSiteName key to be read from a BUFR file, test fail because it's missing from the dummy 'BUFR' dict.

So, as @trygveasp mentioned, if stationOrSiteName is a required key now, then it seems that it should be added to the dictionary in conftest.py to get tests working. Or maybe a correct exception should be thrown when trying to access a missing field?