Skip to content

Ensure T-group temperatures are consistent with previously parsed temperatures#183

Open
akrherz wants to merge 2 commits intopython-metar:mainfrom
akrherz:gh182_temp_conflicts
Open

Ensure T-group temperatures are consistent with previously parsed temperatures#183
akrherz wants to merge 2 commits intopython-metar:mainfrom
akrherz:gh182_temp_conflicts

Conversation

@akrherz
Copy link
Collaborator

@akrherz akrherz commented Jul 23, 2024

My feelings would not be hurt if this PR is rejected by the reviewers here. This PR also includes a related change to update the hopefully only internally used labels to define what the T-group is. It was likely a previous copy/paste as Temp1Hour

closes #182

Copy link
Collaborator

@phobson phobson left a comment

Choose a reason for hiding this comment

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

Looks good!

@akrherz
Copy link
Collaborator Author

akrherz commented Jul 26, 2024

Thanks for the reviews. I am crossing checking my understanding of the world with my METAR archive. One thing that seems clear is that the integer temperature values are truncated and not rounded. ie

KLYO 260855Z AUTO 15004KT 10SM CLR 21/17 A3009 RMK AO2 T02150171

That's why this PR's check does the greater than 1 degree comparison :/

I did a check of all US METAR data for July 2024 and found 33 examples of failures. ie

KBYY 261055Z AUTO 26003KT 7SM RA SCT015 BKN042 OVC100 22/22 A2999 RMK AO2 P0002 T02370232

@tomp
Copy link
Member

tomp commented Jul 27, 2024 via email

@akrherz
Copy link
Collaborator Author

akrherz commented Jul 27, 2024

@tomp Yeah, I am getting cold feet on this PR and instead think we should create a t-group temperature attribute to hold the parsed value and let the end user decide what to do when then they do not agree. Additionally, I asked about this on X and got this poll result
Screenshot from 2024-07-27 10-41-19

@akrherz
Copy link
Collaborator Author

akrherz commented Jul 27, 2024

So, what I a proposing now is

  • Create tgroup_temp and tgroup_dewpt attributes
  • If they are close, assign the tgroup value into the base temp and dewpt values, maintaining current behaviour
  • If they do not agree, emit a warning and keep the values separate

@tomp
Copy link
Member

tomp commented Jul 27, 2024

The current behavior is correct. I might throw a warning if there's a significant discrepancy, but I wouldn't do anything beyond that.

I'm not crazy about adding tgroup_temp and tgroup_dewpt fields. It would be more useful just to report those values in the warning.

@akrherz
Copy link
Collaborator Author

akrherz commented Aug 29, 2024

The current behavior is correct.

I thought that too, but now I am not so certain. Interestingly, the previously used label of Temp1Hour is likely the most correct based on documentation that I have read, but it doesn't really reflect some hour long averaged value.

Now the source of concern is the lack of documentation that states that the t-group temperature is supposed to correspond with the integer C value within the METAR. I can't find this and in fact, the value may not even be coming from the same internal data table within ASOS (at least in the US). The best I can find is that "once per hour, a T-group value should be reported to 0.1 degree C", nothing that says this provides more precision to the what is reported in the integer value

Actual review of METARs in the wild continues to show a horrific story, with these two values not always agreeing. Heaven help us.

@phobson
Copy link
Collaborator

phobson commented Aug 29, 2024

I'll confess I'm not sure what should happen here and the final outcome likely won't affect my use case of METAR data. I'm curious if Ryan May got back to you directly on that tweet though.

@akrherz
Copy link
Collaborator Author

akrherz commented Aug 29, 2024

@phobson , you have a good memory, I don't think he did, but its lame Twitter/X. I'll egregiously tag him here @dopplershift as I know how much he loves issues like this and will hopefully somewhat interesting for what metpy should do.

@dopplershift
Copy link

I remember it, but no clue if I did respond. Right now MetPy isn't parsing the remarks section. Were we to start, my initial thoughts is that we'd have separate columns, with maybe an option to combine them or perhaps a combined column; or maybe the temperature/dewpoint columns would reflect the combined and both raw values would be reported in their own columns.

A warning when there's a discrepancy also seems useful.

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.

METAR remark T-group handler should error when value conflicts with int value provided in non-remark

4 participants