ENH: Support for Product 176 (DPR) - Instantaneous Precipitation Rate #919
Conversation
Added support for reading Nexrad Product 176 (DPR - Digital Instantaneous Precipitation Rate) files. These files share many headers in common with the other products already supported by Pyart, but are in generic radial product format.
Fixed a mistake I made with multiplication of a list.
|
Is this something we'll want to add a new test for? |
|
@dcedgren Thanks for submitting this! I agree, that maybe once the data structure is decided, the dependency can be added. @scollis Would do you think on the dependency part? @dcedgren a test would be preferred to test a Product 176 for this, but I don't think anything crazy as a lot of the nexrad level III is already tested. License looks good to me. |
Added testing for Product 176.
|
Ok, added a test to read in an example DPR file. I just kept the same format as the other tests, so there are more tests than are probably necessary. Fingers crossed I did this all right. |
|
@dcedgren Thanks for adding that! Seems to be failing due to a missing parenthesis syntax error line 202 of test nexrad level 3 |
Fixed syntax error and strengthened reference to Metpy for future.
|
Hmm. I don't think it was a parenthesis. I think it was a period at the end of the line. The prior tests have periods at the end of the function definitions, but I've never seen that before. I just echoed it but I think that's what it didn't like. |
Also added correction in testing for altitude in meters instead of feet.
|
This pull request will fail because instrument altitude in the testing has been changed to meters rather than feet. Once the new feet vs. meter pull request is merged it should pass and as far as I'm concerned should be ready to be incorporated. |
|
Will close PR and reopen to retrigger checks, which should pass now that feet vs. meters discrepancy has been resolved. |
|
Ok, that didn't work. I'll try adding a dummy commit instead. |
|
@dcedgren I'll do a PR to your branch shortly to fix it |
Adding a dummy comment to force checks.
Deleting the dummy comment
|
Weird, it still shows conflicts, but they should have been resolved, is there maybe a marker still hidden somewhere? I don't see anything though which is weird |
|
Hmm. I may not have understood exactly what was going on here. I assumed the checks just had to be rerun, but I guess there is a conflict with another more recent pull request and Github can't figure out how to sort it. Sorry, still trying to learn the lingo. |
Conflict fix
|
There we go, I expect appveyor to fail, but its unrelated to this PR, but once travis passes, we should be groovy |
|
pyart/io/tests/test_nexrad_level3.py:196: AssertionError I don't understand why Travis failed. |
|
Weird... They match if i'm reading that correctly lol, im going to restart the build |
|
So it's the last line of the test, and of the module, that fails. That makes me suspicious, but I don't know what could cause that. Also, how did you restart the build? That was what I was trying to do earlier. |
|
I'm not sure either, it looks like it shows the values matching, which is odd. To restart the test, you go into details for the testing such as travis on this page. And once your GitHub is linked to travis, after going to the details page in the top right, there is a retart build option |
Asserting data value for test_nexrad_level3_msg176() was failing because a float was being compared to a np.float32 value. It was working where there was no extra precision but for this new test case there were extra trailing digits so somehow the rounding wasn't working.
|
Ok, Travis passed. It should be good to merge. |
|
Thanks again for doing this @dcedgren ! Merging. |
|
Absolutely! It's my first real open-source contribution and it's been fun. Thanks for your help. |
Added support for reading Nexrad Level III files of Product 176 in accordance with discussion in Issue #551.
I relied heavily on the Metpy implementation to read the Generic Radial Product format. But as I got into it I decided I was close enough to understanding the Pyart implementation that I could probably modify it with only short sections of additional code (as opposed to, say, reading in the file using Metpy and assigning bits and pieces of dictionaries and lists to line up with a pyart.core.radar object). It will be more maintainable this way.
In general I left the existing code and made product 176 work alongside it. Nothing of substance has changed for the code to read other products in.
The key piece I used from the Metpy implementation I used was the Level3XDRParser class. I went back and forth about whether or not to add Metpy as a dependency and import the class or to copy the 100-150 lines of code into Pyart. In the end I decided to copy them since per Dopplershift In Issue #551 a final data structure has yet to be decided for Metpy and it seems possible something could break here in the future. I think it seems reasonable if the maintainers want to go the other way instead. Also check that I did the license thing right.