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

set default METAR year, month from UTC date #2701

Merged
merged 1 commit into from
Oct 7, 2022

Conversation

akrherz
Copy link
Contributor

@akrherz akrherz commented Oct 1, 2022

closes #2700

Description Of Changes

METAR is a horrible format that provides just a UTC day, so we have to make life choices to figure out what the month and year is for this day. MetPy previously assumed a default year and month matching the current system localized date and not the current UTC date.

Checklist

No idea how this can be tested outside of doing naugthy things to python's datetime module?

@akrherz akrherz requested a review from a team as a code owner October 1, 2022 03:15
@akrherz akrherz requested review from dcamron and removed request for a team October 1, 2022 03:15
Copy link
Member

@dopplershift dopplershift 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 finding and fixing. Is there any reason you didn't use datetime.utcnow() instead?

I can't think of any way to test realistically.

@dopplershift
Copy link
Member

I stand corrected. If we monkey patch time.time() to return what we want, that will affect things the right way. I can push the test if you have no clue what I'm talking about.

@akrherz
Copy link
Contributor Author

akrherz commented Oct 1, 2022

Is there any reason you didn't use datetime.utcnow() instead?

I did, but then I read the python docs and it told me the preferred way.

@akrherz
Copy link
Contributor Author

akrherz commented Oct 1, 2022

If we monkey patch time.time() to return what we want, that will affect things the right way. I can push the test if you have no clue what I'm talking about.

I can likely try that later tonight or if you want to do it, go for it.

@akrherz
Copy link
Contributor Author

akrherz commented Oct 2, 2022

Eh, @dopplershift, I could not figure it out. It also gets tricky as one has to ensure you have a situation of the current datetime (timezone could be anything around the world when this code runs) is for the previous month. I tried setting the TZ os.environ variable for this purpose, but it wasn't doing what I hoped either...

@dopplershift
Copy link
Member

Looks like datetime is going to be uniquely problematic to monkeypatch for testing, so we can just put this in.

@dopplershift dopplershift added Type: Bug Something is not working like it should Area: IO Pertains to reading data labels Oct 7, 2022
@dopplershift dopplershift added this to the September 2022 milestone Oct 7, 2022
@dopplershift dopplershift merged commit d0e2fa6 into Unidata:main Oct 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: IO Pertains to reading data Type: Bug Something is not working like it should
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug with 'date_time' not reading the next month from METAR properly
2 participants