vtsuperdarn / davitpy

DEPRECATED The DaViT Python project
http://vtsuperdarn.github.com/davitpy/
GNU General Public License v3.0
37 stars 59 forks source link

FoV plotting bug hotfix #324

Closed aburrell closed 6 years ago

aburrell commented 6 years ago

Field point calculation at large distances was breaking FoV plotting, since the more distant locations weren’t meeting the conditions for reliability. I fixed this by adding a flag that defaults to skipping this evaluation. I would recommend setting this flag to True when performing studies where one actually cares about the location, but this level of accuracy is not what we're looking for when making FoV plots.

aburrell commented 6 years ago

TEST based on issue #321

import davitpy import matplotlib.pyplot as plt import datetime as dt plt.ion() stime = dt.datetime(2016,6,1) f = plt.figure(figsize=(16,8))

ax1 = f.add_subplot(121) m1 = davitpy.utils.plotUtils.mapObj(boundinglat=30.0, gridLabels=True, ax=ax1) davitpy.pydarn.plotting.overlayRadar(m1, fontSize=8, plot_all=True, markerSize=5) davitpy.pydarn.plotting.overlayFov(m1, plot_all=True)

ax2 = f.add_subplot(122) m2 = davitpy.utils.plotUtils.mapObj(boundinglat=-30.0, gridLabels=True, ax=ax2, datetime=stime) davitpy.pydarn.plotting.overlayRadar(m2, fontSize=8, plot_all=True, markerSize=5) davitpy.pydarn.plotting.overlayFov(m2, plot_all=True)

fov_bug_map

scivision commented 6 years ago

As a note for future modernization, there is a Python standard library syntax for declaring type hints. That is, to declare the type of a variable.

def func(a:int, b:float) -> float:

MyPy is a powerful linter using these type hints. PyCharm IDE uses type hints.

In the future, JIT compilers may use these type hints.

asreimer commented 6 years ago

It works! But I don't want to merge until #312 is merged.

aburrell commented 6 years ago

Since #312 is being reformated, I think we should merge this in. Thoughts, @asreimer ?

aburrell commented 6 years ago

@asreimer , @ksterne shall we merge this in? Not great to have bugs lying around.

ksterne commented 6 years ago

Sorry for the delay here @aburrell! Lots of other things taking up my time. Maybe I can work on this this week.

scivision commented 6 years ago

Yes merge is my opinion. I will make my (unrelated) changes in smaller pull requests

ksterne commented 6 years ago

Hey @aburrell, I'll give my :( for different remote switching. But I know you usually operate out of your own repo. pr324_after_pgrrkn While it looks better, there's still some issues with plotting pgr and rkn here. There might be one other one, but I can't see it quickly. Do you know off hand what's causing that or is it easy to look into? If it's outside the scope of disabling the fine accuracy for locating scatter, then I'm seeing the same as you are. pr324_after

aburrell commented 6 years ago

From what I remember, those are the radars that have a maxgate of 225. So, if you don't specify a maximum range gate of 75 or 100 or so, they take over a lot of the globe. If you limit their range gates, they should look sensible.

ksterne commented 6 years ago

Ah, right! I always forget those values that were put in and makes things a little strange. One day it might be neat to change the maxGate parameter to a list that follows the radar codes/IDs. But that's another day for sure. Anyways, using the maxGate parameter everything looks sensible for me! @asreimer? Or anyone else?

aburrell commented 6 years ago

Ok, looking back at the comments now that I've had several cups of tea, it looks like @asreimer and @ksterne have tested. Would one of you like to merge?

ksterne commented 6 years ago

I can get to it, but not until tomorrow. If @asreimer does before me then no problem.

ksterne commented 6 years ago

OK, getting to merge this now. @aburrell, do you want to make the PR to pull this to develop as well?

aburrell commented 6 years ago

Added on #344 .