-
Notifications
You must be signed in to change notification settings - Fork 21
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
Updated documentation, ROI, and file reading. #36
Conversation
Instituted a modified version of the Region of Interest PR #34. Added save, open, and reset capabilities of ROI selection. In addition added a Table output window for ROI selection. Updated the restore_to_default in tools.py to be more robust as more tools are added. RHI and airborne file types are now working once again.
This looks good, for me it ok merging it. |
|
||
# Check the file type and initialize limts | ||
# Create a figure for output | ||
self._check_file_type() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't need _check_file_type here, newRadar does it and will be executed in line 126
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll need to look at this, but I know the _set_fig_ax function needs _check_file_type to be run first to get the proper limits set. In turn the LaunchGUI requires the all of this to be done so that the "canvas" can be set up properly.
That is why it has persisted in two locations. Maybe there is an elegant way to treat this problem, but I haven't figured it out yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so there is a conflict between initialization and execution, this indicates we are doing something wrong.
From the start: when should the limits be reset?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The _initialize limits need to be set in three circumstances:
- before drawing the initial plot
- when the plot type (i.e. ppi goes to rhi) changes
- when the reset default is chosen.
This causes a "hard" reset to a default state.
Maybe this could be broken into a couple of different functions. The reason the figure needs it is to define the aspect ratio of the plot (RHI and Airborne both have longer X than Y axes). This was convenient a while back, but could probably be changed now.
Renamed plot.py to plot_radar.py
@gamaanderson I renamed plot.py to plot_radar.py and made some structural changes per your line of thought. Basically I create figure first and then modify it later from withing the check_file_type. This allows this operation to be performed with NewRadar, taking care of all three instances I believe. |
@@ -648,7 +605,7 @@ def _check_file_type(self): | |||
'''Check file to see if the file is airborne or rhi.''' | |||
radar = self.Vradar.value | |||
if radar.scan_type != 'rhi': | |||
return | |||
pass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reset flags
self.rhi = False
self.airborne = False
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to be explicit here.
Removed plot size from limits. Plot size is explicitly set in plot_radar using checks for scan type. Scan types are now handled better and limits are properly set when changing scan types.
Instituted a modified version of the Region of Interest PR #34.
Added save, open, and reset capabilities of ROI selection.
In addition added a Table output window for ROI selection.
Updated the restore_to_default in tools.py to be more robust as more tools are added.
RHI and airborne file types are now working once again.