Time-height displays. #453
Conversation
Updated datetime_utils to convert to Epoch time if desired. Updated RadarDisplay.plot_vpt to accept a flag to use time along x-axis. Added time formatting for x-axis for time-height plots.
|
Just FYI, I used a NEXRAD L2 file for test. Read in, converted using |
| @@ -1019,6 +1019,7 @@ def join_radar(radar1, radar2): | |||
|
|
|||
| # to combine times we need to reference them to a standard | |||
| # for this we'll use epoch time | |||
| ### NG THIS should use new updated datetime_utils if accepted | |||
There was a problem hiding this comment.
Typically XXX to TODO are used to mark areas of code where future improvements might be needed. This makes these areas easy to grep for.
|
@nguy Thanks for this addition, having the ability to plot times in VPTs will be great. I left a few comments on some places where I think the code could be improved. |
Updated doc strings in radardisplay.py Updated test script.
Moved epoch units to a variable in datetime_utils, but maintained a get method to pull the units out if needed. Code in radar.join_radars is added, but still commented out for the time being.
| @@ -35,6 +35,7 @@ | |||
| from ..config import get_metadata | |||
| from ..lazydict import LazyLoadDict | |||
| from .transforms import antenna_vectors_to_cartesian, cartesian_to_geographic | |||
| from ..util import datetime_utils | |||
There was a problem hiding this comment.
This is going to cause a circular import problem if anything in the util sub-package imports from core, which the xsect module does. For the time being I would remove this import and the commented out line but keep the TODO line
There was a problem hiding this comment.
Ooh, that's not good. Removed.
|
@nguy, A few additional comments. I would keep any changes to core/radar.py independent of this PR as there are some special concerns that need to be considered when making changes to anything in the core sub-module. Sorry to be a bugger on some of the style issues, I really like this new feature, but want to keep Py-ART's style consistent. |
|
@jjhelmus No worries. It's best practice to follow the same style. Sloppy on my part. Removing those lines from radar.py should remove any changes to the core, excepting the not for potential future changes. Is there a reason that the |
Removed commented out code from pyart.core.radar. Bug fix in radardisplay having to do with unused keyword. Additionally, removed datetime calculation of time variable as an attribute of RadarDisplay.
|
I do not believe there is any good reason that |
|
Do you want me to do that as part of this PR or leave it for another? |
|
Do the move in another PR, makes it easier to find and revert if necessary. Since join_radar is not in the user docs I don't see an issue with renaming it without an alias. The alias can always be added in later if folks request it. I think this PR is good to merge. I'll wait util the CI checks finish, just to be sure. |
|
Sounds good. Do you want me to just do that as a mini-PR after we merge this? |
|
Shoot, I forgot to remove the |
|
Go for it, I can hold off. |
|
A mini-PR to move |
|
I just started the push. Should be done momentarily. |
|
Sounds like a plan. Just waiting to CI to pass, then I'll merge this. Thanks for all you work on this! |
|
No problem. Luckily this was an easy one since it only took porting the code. |
Updated datetime_utils to convert to Epoch time if desired.
Updated RadarDisplay.plot_vpt to accept a flag to use time along x-axis.
Added time formatting for x-axis for time-height plots.
This is a suggestion at implementing the time-height plots discussed in Issue #451 and also allows to conversion of Epoch time for future development (perhaps Issue #435 ?).