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

Correct section 3 and 4 mappings + more #49

Merged
merged 20 commits into from
Mar 8, 2024
Merged

Correct section 3 and 4 mappings + more #49

merged 20 commits into from
Mar 8, 2024

Conversation

RoryPTB
Copy link
Contributor

@RoryPTB RoryPTB commented Mar 5, 2024

Summary of changes made:

  • The section 3 and 4 replicated cloud groups are now correctly encoded to BUFR.
  • The section 3 precipitation time period defaults to the last 3 hours unless otherwise stated, as suggested by ECMWF.
  • Update of GitHub actions.
  • Other small bug fixes.

# Check if the beginning of the message, that we're about to throw
# away (data[0]), also contains AAXX and thus there must be a
# typo present at the AAXX YYGGiw part of the report
if data[0].__contains__("AAXX"):
Copy link
Member

Choose a reason for hiding this comment

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

"AAXX" in data[0]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since the message is split around AAXX YYGGiw (with an indefinite number of spaces between AAXX and YYGGiw as sometimes there is an extra space by accident), data[0] would be the part of the message before this section (e.g. ZCZC 123 SMCU20 MUHV 310000). This is normally discarded, but if it contains 'AAXX', then that means the splitting of the message has gone wrong due to a typo somewhere. There was a bad piece of data that motivated this change but it's been so long now I can't remember.

Copy link
Member

Choose a reason for hiding this comment

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

I understand that, but why are we using __contains__ rather than in?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are equivalent but I will change it to in, I can't remember why I put __contains__.

@@ -1544,6 +1557,21 @@ def transform(data: str, metadata: str, year: int,
# Now we need to add the mappings for the cloud groups
# of section 3 and 4
try:
# Define a new method which handles the updating of
# the mapping file with section 3 and 4 cloud data
def update_data_mapping(mapping: list, update: dict):
Copy link
Member

Choose a reason for hiding this comment

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

why is this embedded in the try block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No particular reason. It was an oversight to put that closure in the try block since the code immediately after must be, in case no station height is provided for the station in question.

@RoryPTB RoryPTB merged commit 222e243 into main Mar 8, 2024
10 checks passed
@RoryPTB RoryPTB linked an issue Mar 8, 2024 that may be closed by this pull request
@RoryPTB RoryPTB deleted the rory-s4-fix branch March 8, 2024 09:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants