Skip to content

fix: add code translation to no significant weather, nsw#178

Merged
akrherz merged 2 commits intopython-metar:mainfrom
JordonMaule:main
Aug 31, 2023
Merged

fix: add code translation to no significant weather, nsw#178
akrherz merged 2 commits intopython-metar:mainfrom
JordonMaule:main

Conversation

@JordonMaule
Copy link
Contributor

@JordonMaule JordonMaule commented Aug 30, 2023

Code tested: VEIM 301200Z 16007KT 7000 NSW SCT018 31/27 Q1007 NOSIG
Before:
image

After:
image

@phobson
Copy link
Collaborator

phobson commented Aug 30, 2023

Thanks for PR, @JordonMaule . Do you happen to know if any other keys are missing from that dictionary?

Could you add your example as a test in the test suite?

@JordonMaule
Copy link
Contributor Author

JordonMaule commented Aug 31, 2023 •
edited
Loading

@phobson Thank you for your quick reply.
I believe the 'other' codes are all covered.

Most times NSW appears on the end of the bulletin, and is not recognized. (Haven't had the time to review this yet)
But if it's on the middle of the bulletin it matches with the Regex from line 77, and we didn't have the key on WEATHER_OTHER.

I've added tests to all the keys in WEATHER_OTHER, if you'd prefer to have only the NSW code tested let me know and I'll remove the others.

@phobson
Copy link
Collaborator

phobson commented Aug 31, 2023

@JordonMaule thanks for the test, looks great!

@codecov-commenter
Copy link

codecov-commenter commented Aug 31, 2023

Codecov Report

Merging #178 (0b814bf) into main (e878b1c) will not change coverage.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the GitHub App Integration for your organization. Read more.

@@           Coverage Diff           @@
##             main     #178   +/-   ##
=======================================
  Coverage   88.52%   88.52%           
=======================================
  Files           4        4           
  Lines        1046     1046           
=======================================
  Hits          926      926           
  Misses        120      120
Files Changed Coverage Δ
metar/Metar.py 90.19% <ø> (ø)

@phobson phobson requested a review from akrherz August 31, 2023 07:45
akrherz
akrherz approved these changes Aug 31, 2023
View reviewed changes
Copy link
Collaborator

@akrherz akrherz left a comment

Choose a reason for hiding this comment

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

Thanks for improving the tests!

@akrherz akrherz added this to the 1.11.0 milestone Aug 31, 2023
@akrherz akrherz merged commit b96e901 into python-metar:main Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akrherz akrherz akrherz approved these changes

@phobson phobson phobson approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

1.11.0

Development

Successfully merging this pull request may close these issues.

4 participants

@JordonMaule @phobson @codecov-commenter @akrherz