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

Update bufr.py for carra2 for local observations #35

Merged
merged 7 commits into from
Jan 24, 2024

Conversation

PerDahlgren
Copy link
Contributor

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

@trygveasp
Copy link
Collaborator

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

pysurfex/bufr.py Outdated
@@ -1,5 +1,6 @@
"""bufr treatment."""
import logging
import sys
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is not needed?

pysurfex/bufr.py Outdated
try:
value = self.td2rh(td2m, t2m)
value = value * 0.01
value = value * 0.01
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove the white-spaces

@trygveasp
Copy link
Collaborator

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
Copy link
Contributor Author

PerDahlgren commented Jan 16, 2024 via email

@PerDahlgren
Copy link
Contributor Author

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
Copy link
Collaborator

joewkr commented Jan 18, 2024

@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?

@trygveasp trygveasp merged commit 4919267 into metno:master Jan 24, 2024
2 checks passed
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.

3 participants