Quick fanplot fix - Githubissues

mts299 commented 6 years ago

Here are the quick and dirty fixes for the following issues:

basemap depreciation with using patches => updated it to version 1.1.0
matplotlib depreciations => set matplotlib == 2.0.2 to avoid depreciation warnings from basemap
plotFan matplotlib error => fixed
reading in data from a file and passed to plotFan (Improvement)=> implemented
"None" data bug: if the radarDataOpen returns None then the radar list length is different in length to the data records (myFiles) and this causes an index out of range in the plotting step. => Fixed by removing the radar name from the list if there was no data provided for that radar.

Unfortunately, the code is in bad shape (for python standards) and without changing too much, this is all I could for now. I have another branch I am working on to try and put davitpy in better shape.

Please test it out :) and hopefully, it doesn't break other plotting methods!

mts299 commented 6 years ago

Do we want this merge to develop or to master? I noticed @aburrell fix for the FOV points was in the master branch but not the develop branch. I could make another branch off of develop and include @aburrell fix and my other fixes if we prefer.

aburrell commented 6 years ago

That was because it was requested as a hotfix. There's a pull request to put this into develop that no one has tested yet, and that's why it's not included. ( #344 )

mts299 commented 6 years ago

Okay, well I can take a look and test it out for you :) Then I can add my changes as well.

aburrell commented 6 years ago

That would be great! Let’s share the testing load :)

On 11 Apr 2018, at 19:59, mts299 notifications@github.com<mailto:notifications@github.com> wrote:

Okay, well I can take a look and test it out for you :) Then I can add my changes as well.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/vtsuperdarn/davitpy/pull/354#issuecomment-380641221, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AGuC_hQ3zzjqLrSTbhyhdZSvKcfFIwbnks5tnqb1gaJpZM4TO3l1.

aburrell commented 6 years ago

@mts299 , do you have some tests I can run to see how well this is working? I can take a look at this today.

mts299 commented 6 years ago

I do have a script I was using to generate the fan plots. What would be the best method to give it to you?

aburrell commented 6 years ago

Will github let you attach it here? Then the second tester (since we need two to merge) will have access to it as well. If not, here's my email: agb073000@utdallas.edu

mts299 commented 6 years ago

It looks like I cannot directly attach the script to github :( I will email it to you, @aburrell and @ksterne. In the future, I will be putting these script onto our github page so that could be a place to get scripts to test davitpy with.

ksterne commented 6 years ago

We didn't answer the question here about whether or not this should go into master or develop. I'd think to merge into develop in prep for a release for the workshop? It doesn't seem like it's a pressing issue, at least from what I can tell.

aburrell commented 6 years ago

I agree that it should go into develop, @mts299 do you mind redoing this request into develop?

mts299 commented 6 years ago

Changed :)

aburrell commented 6 years ago

Here is the code I used to test (and something to use as a template for future pull requests). This has benefits over the script in that everyone can see it (and it shouldn't have to be altered for it to run on any local system, though there's usually something that pops up on that front).

import davitpy
import datetime as dt
import matplotlib.pyplot as plt
rad_codes = ['sas', 'han']
stime = dt.datetime(2007, 1, 1)
etime = stime + dt.timedelta(hours=1)
# I set rcParams to download data from VT
radar_data = list()
for rad in rad_codes:
    radar_data.append(davitpy.pydarn.sdio.radDataOpen(stime, rad, eTime=etime,fileType='fitacf'))
    print radar_data[-1]._radDataPtr__filename
# Make the plot
param='velocity'
davitpy.pydarn.plotting.fan.plotFan(stime, rad=rad_codes, myFiles=radar_data, param=param, gsct=True, show=False, png=True)

with output:

WARNING:root:setting dateTime to datetime
WARNING:root:setting dateTime to datetime

/tmp/sd/20070101.000000.20070101.010000.sas.fitacf
/tmp/sd/20070101.000000.20070101.010000.han.fitacf
---------------------------------------------------------------------------

And the rest of the debug output when it died:

IndexError                                Traceback (most recent call last)
<ipython-input-46-b4872ddfbb35> in <module>()
----> 1 davitpy.pydarn.plotting.fan.plotFan(stime, rad=rad_codes, myFiles=radar_data, param=param, gsct=True, show=False, png=True)

/Users/ab763/Library/Python/2.7/lib/python/site-packages/davitpy-0.8-py2.7-macosx-10.12-x86_64.egg/davitpy/pydarn/plotting/fan.pyc in plotFan(sTime, rad, interval, myFiles, fileType, param, filtered, scale, channel, coords, colors, gsct, fov, edgeColors, lowGray, fill, velscl, legend, overlayPoes, poesparam, poesMin, poesMax,     poesLabel, overlayBnd, show, png, pdf, dpi, tFreqBands)
    393         while(allBeams[i] is not None and allBeams[i].time < bndTime):
    394             # filter on frequency
--> 395             if (allBeams[i].prm.tfreq >= myBands[i][0] and
    396                     allBeams[i].prm.tfreq <= myBands[i][1]):
    397                 scans.append(allBeams[i])

IndexError: list index out of range

So, it looks like it dies because multiple radars were used, but multiple frequency bands were not. Given that this is what your script does (but your script uses more radars), I'm a bit surprised... is that something you've encountered? Do you consider it inside or outside the scope of this pull request?

If I use a single radar, it works. That radar has to be part of a list, but that's something we can fix in another pull request.

Using just 'sas', I get:

sas_fan

mts299 commented 6 years ago

@aburrell - So here is the interesting part of your script minus a typo: etime = stime + dt.timedelta(hour=1) should be hours or else timedelta gets cranky etime = stime + dt.timedelta(hours=1)

It didn't crash for me... I got an image with both radars showing: 20070101 0000 60 fan

And when I run my script I do get multiple radars: 20150317 0000 60 fan

However, you are correct that they do not show multiple frequencies and Kevin and I noticed this same problem with RTI plots when it says there is no data in between different frequency ranges,

ERROR:root: No data found in frequency range 8000 kHz to 20000 kHz

But there is data. Not sure if these issues are related or separate. But this may suggest a different issue to be created as it might be affecting other plots.

Also, I think the scope of this pull request was to fix the matplotlib error and get it to read in from the file. Multiple radars I think was thought to be already implemented but I guess that feature might also have some bugs in it.

I am curious why you cannot get it to work.

aburrell commented 6 years ago

Nice catch on the timedelta, I rarely remember which one it is until I guess right or wrong (one of the reasons I usually define stime and etime separately). Test code above has been edited.

aburrell commented 6 years ago

However, you are correct that they do not show multiple frequencies and Kevin and I noticed this same problem with RTI plots when it says there is no data in between different frequency ranges,

ERROR:root: No data found in frequency range 8000 kHz to 20000 kHz

But there is data. Not sure if these issues are related or separate. But this may suggest a different issue to be created as it might be affecting other plots.

Also, I think the scope of this pull request was to fix the matplotlib error and get it to read in from the file. Multiple radars I think was thought to be already implemented but I guess that feature might also have some bugs in it.

It looks like I found another issue. If you want to limit the scope of this pull request to part of the problems, that's fine by me (there are a lot of problems).

mts299 commented 6 years ago

What is the other issue?

My biggest concern is that you cannot produce plots but I can?

Yea I think if we try to fix all the issues in one go it might become a bit overwhelming :joy:

asreimer commented 6 years ago

I modified @aburrell 's comment above so that it's easier to copy paste the test code used. Then I ran it and obtained the same plot as @mts299, the one with 2 radar FOVs, the left one with data, the right one missing data.

Next, I ran a script. Rename this file to have a .py extension and run: python generateSDplots.py

If you do, you get the same plot that @mts299 did with the 5 FOVs. I modified the version that was emailed to me and @aburrell so that it works by fetching data from the VT server instead of locally.

asreimer commented 6 years ago

I have a question. See: https://github.com/vtsuperdarn/davitpy/pull/354/files#r184900504

I'm good with merging this once my question has been addressed.

mts299 commented 6 years ago

Because basemap will throw depreciation warnings if matplotlib is too new. There was talk about moving away from basemap so I figured this would be a quick fix.

ksterne commented 4 years ago

Looks like we didn't get around to fully getting this in. Closing this since this repo is being deprecated for now.

vtsuperdarn / davitpy

Quick fanplot fix #354