Add parameter to initialize partially parsed METARs #25
Conversation
Offer a way to convert ParserErrors into warnings when minor information was not understood.
|
@tomp do you have any plans to merge this? |
|
I don't think this is helpful, unfortunately. If there are unparsed groups in the METAR code, it's an error. The proper way to handle an error like this in Python is to throw an exception. If you think the generic exception that's currently thrown should be more specific, I'd be open to creating a more specific exception for the case that you want to be able to ignore. One way or the other, the correct way to handle this is for you (the client) to catch my exception, rather than for me to suppress the error. |
|
Under some circumstances we are only interested in some partial information of the METAR. In my project e.g. I was only interested in cloud cover. I did not care whether the wind was correctly parsed (see e.g. here: https://github.com/tomp/python-metar/issues/26). But your exception handling would have forced me to totally ignore that measurement or alternatively to write my own parser. In the readme you say that some local formats are not supported and as I am not an expert on METARs I was not sure which of these issues were linked to that fact. So I believe that it is not necessary to see it as an error if some details could not be parsed. If those details are not the ones you are currently interested in there might be nothing bad about it. |
|
@tomp this decision makes usage of your library difficult. In general, the exception causes no valid METAR object reference to be created, so one has no option for even partial METAR parsing. My workflow currently is to:
If you were fully confident that your library implements the entire METAR specification (one may argue that such a thing even exists), the this type of disciplined exception raising would be valid IMHO I think providing the user the option (disabled by default as the PR does) is a good idea and doesn't change your current default behaviour decision. |
|
Just came across this METAR from aviationweather.gov:
That silly 'M' there is messing up everything for me. In my case, I am loading up all of the metars I have for a given hour into a Pandas Dataframe. I want my dataframe to record that there is a maintence indicator on, and that there are some other things turned off. Right now, I am completely blind to the fact that this station is broken and putting out bad metars in my code. |
|
Also, as a side note, I would suggest modifying this implementation slightly so that a flag is set such as "self.is_valid = False" or something, in addition to emitting the warning. |
|
@WeatherGod Feel free to take my changes and adjust them as another PR. The best way to express the idea (which is still a bit vague to me) is to show it. |
|
perhaps I might do that... I have to be careful not to let myself down a rabbit-hole of a bunch of little changes I have been thinking of, particularly to allow for others to easily extend the parsing capabilities by making a subclass, or passing in a list of additional regexs/function tuples. But the basic idea with what I was saying is that after the object creation, I should be able to check an attribute to know if the metar is completely valid or not. Warnings, while useful, are more intended for logs. |
|
Oh, and for those who might be interested in a possible work-around (I do deeply apologize for the crass language for this package name and its documentation, but it does work): https://pypi.python.org/pypi/fuckit |
|
I am curious how you intend to solve it, looking forward to it! |
|
Hi @WeatherGod , the |
|
@akrherz With the current project velocity of this project using metpy is the better way to go I guess. |
|
The `is_valid` name was just a suggestion. Maybe `incomplete_parse` might
be a better name?
Metpy is trying to position itself as a replacement to gempak. Last time I
checked, gempak doesn't decode metars, so I doubt it'll adopt it. Thoughts,
@dopplershift?
I will say this, at this particular moment, my employer requires a metar
decoder in python that is capable of decoding info from aviationweather.gov.
Now, how far he wants me to go with this, I am not sure (he might just want
me to drop these "invalid" metars and move on), but it could mean that I'll
need to fork this package.
…On Thu, Oct 12, 2017 at 8:47 AM, 1kastner ***@***.***> wrote:
@akrherz <https://github.com/akrherz> With the current project velocity
of this project using metpy is the better way to go I guess.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/tomp/python-metar/pull/25#issuecomment-336122879>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AARy-E6aF66Wd_t8MFAZGF4ZWLr64SDdks5srgpUgaJpZM4NaBQd>
.
|
|
@WeatherGod GEMPAK has decoded metars for a very long time :) |
|
Ah, good to know. The last time I regularly worked with gempak was in
undergrad, and that was mostly for learning about potential vorticity and
eta levels...
…On Thu, Oct 12, 2017 at 9:31 AM, daryl herzmann ***@***.***> wrote:
@WeatherGod <https://github.com/weathergod> GEMPAK has decoded metars
<https://www.unidata.ucar.edu/software/gempak/man/prog/dcmetr.html> for a
very long time :)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/tomp/python-metar/pull/25#issuecomment-336137989>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AARy-EMXkjXy7c4I8x7WWVltuLF9sPldks5srhS8gaJpZM4NaBQd>
.
|
|
I created #36 as an initial step. |
|
I also created #37, which is a tiny change that should make it possible for users to modify the set of regexes and handlers to use for parsing. See the example there for some ideas. |
|
FYI, my plan is to implement a true parser for METARS, as in true CS parser, rather than abusing regexes for human-coded text: Unidata/MetPy#70 |
Offer a way to convert ParserErrors into warnings when minor information was not understood.