nguy / artview

ARM Radar Toolkit Viewer
http://nguy.github.io/artview
BSD 3-Clause "New" or "Revised" License
48 stars 21 forks source link

Replotting fails when switching scan types #38

Closed nguy closed 9 years ago

nguy commented 9 years ago

With the previous PR #36 some improvements were implemented in the plot function. ARTView now loads the correct default limits. It also produces the correct plot. The issue is that the previous plot is not removed. I've tried multiple erase methods with no luck. I'm stumped right now.

ARTView does plot correclty on startup for PPI, RHI, and airborne.

gamaanderson commented 9 years ago

related to this, you know Display is never getting rhi and airborne flags? the comand line options --rhi and --airborne are useless. Shouldn't we remove those?

nguy commented 9 years ago

I was wondering about this. Yeah I would agree that these are no longer needed. The check seems pretty robust at this point.

nguy commented 9 years ago

This has been improved with PR #42 I was able to switch from PPI to RHI. But I was not able to switch back to PPI again.

gamaanderson commented 9 years ago

I found no problem, what exactly did you experience?

nguy commented 9 years ago

Hmmmm, well I realized I had only tested in one direction. And now I can't recreate the problem...

But we'll have to loosen the constraints of sweep_numbers for tilts in Vradar, because this causes a failure in NOAA P-3 tail radar (airborne) files because there are no sweep numbers (None value).

gamaanderson commented 9 years ago

Well, sweep_number is a mandatory cfradial variable, so it should be safe to use it (I assumed). Are this radars writable, I mean does pyart.io.write_cfradial work?

nguy commented 9 years ago

Yes it's safe to use, but I bet we can add a check for None value and maybe scan_type to check if it is an airborne radar.

gamaanderson commented 9 years ago

Well, I'm lost, but if you think it is necessary. I just wish it was not necessary, what do I put in Vtilt if there is no sweep? and is every one (classes and functions) safe?

nguy commented 9 years ago

So with that type of data there is technically one sweep per file. The radar antenna rotates continuously during flight (if on) and the data is broken into a single file for each 360 degree rotation. This gets read in by PyArt as a None value then for sweeps. Therefore there are no tilt angles. We could simply set the Vtilt.value to None, which may require a few more housekeeping checks. Or if we checked fo this file type we could fill with either pyartradar.fixed_angle['data'][0] or pyartradar.tilt['data'][0].

gamaanderson commented 9 years ago

So, in theory there is one sweep. What variable exactly is equal to None? may be we should think if PyArt behaviors is the best one. From my understanding, the whole purpose of Radar is to unify things to avoid the need of some tests, making it more user-friendly

nguy commented 9 years ago

Hmmm, I've been looking into this and I'm not convinced it's a PyArt problem. Trolling through their code, it looks like files are being read in correctly. The None error occurred when for some reason a file failed to be read. This would happen after a few rounds of "Open file", but seemingly random? The same file can be read in by PyArt if I go to the directory and just do command line read.

In conjunction I noticed that the rTilt index is getting wacky (for example at one point a value of 2714 was tried as a sweep index!). I'm trying to track down what (and where) is going on here.

So there are two potential problems going on here.

  1. Files are sometimes not read in.
  2. The sweep index is incorrect.

Can someone else just try opening a bunch of files one after another using File->Open and see if they can recreate this problem?

nguy commented 9 years ago

I also just noticed that just opening a bunch of files in a row seems to lock up the display. You can "unlock" by choosing a different field for example....

gamaanderson commented 9 years ago

Couldn't recreate the problem, tested opening 15 ppi files in a role. If it improves you can add the following in the exception, so you can see pyart error message:

            import traceback
            print traceback.format_exc() 
nguy commented 9 years ago

Okay thanks. I may have to try this to get a handle on what is happening.

nguy commented 9 years ago

Okay, I tracked this down finally. It had nothing to do with the sweep_number as mentioned above, but was not getting a Vradar instance.

In the menu.py Menu class, PyArt was failing to reading in Sigmet files with delay_field_loading=True. I guess I had not updated to the latest PyArt. So this brings a question up:

Do we require that PyArt then be the latest (as of now) or do we put in a handler to work just in case the user has an older version? Thoughts @gamaanderson @jjhelmus @tjlang @pfhein?

gamaanderson commented 9 years ago

I believe the main branch can use PyArt feature as soon as they are available, however when making releases we must fix what in the minimum PyArt. As we are already ahead of 1.4, I would suggest when PyArt launches 1.5 we stop actualizing PyArt for a moment just til we have our release done. This is related to issue #44

About handler I don't think we need it for now as there are no external users that I know. In the future when we find a (serious) bug we may not only have to fix it in the Master, but also supply fixes for the other versions as well, of course we must define which are the versions we support for bug fixing. In the same way we must define which are the dependence versions we support, as issue #47 shows we must still think about python 2.6 and some really old version of PyQt, not just because we want, but because RedHat still uses it.

nguy commented 9 years ago

I think that is a good idea to stop actualizing PyArt or just maintain 1.4.x until the release is done.

So I just found out that we do have a few people using the software. The latest is experimentally in the NSF DOWs. So we should consider current users as well.

I agree, some of those may be hard to track down. We can first make a minimum known version list and then current workarounds. After that we can tackle older versions - and in some case I think we require a minimum. We don't have the funds or time to make all version work, unfortunately.

gamaanderson commented 9 years ago

Nice to know we have users, I would love to hear from them what they want from ARTview, I fill we could process new tasks pretty fast if they need something more specific.

In fact we don't have the possibility to test all versions, however when we say we support a configuration we are actually saying we will look after any problems some one may have in this configuration, we (nor anyone else) can never guaranty there are no bugs hidden somewhere. Just for reference I pretty much follow Debian stable (at Jessie now)

pfhein commented 9 years ago

I agree that we need to maintain a minimum version, but I am not certain how to do with the rolling updates of PyART. My last update of PyArt was April 15, so I am a little behind. Thinking about old versions, I wonder about supporting Python 2.6 and all it's old PyQT4 libs. Python 2.7 was released 5 years ago, RHEL 6 was released about 4.5 years ago. Python 2.6 and 2.7 are both feeling a little old and have their problems (issue #47), but I have been running my RHEL 6 clone for only 3.5 years and I am not feeling ready to upgrade (and I know someone still running CentOS 5). RedHat and clones have "software collections" that have Python 2.7, 3.3, and 3.4 available, so an "official" upgrade is possible. On the other hand, PyART does support Python 2.6, 2.7, 3.3, and 3.4, so maybe we should too. RHEL 6 is going to be around for awhile longer. I guess I don't know what to do.

nguy commented 9 years ago

Yes, I'll try to get in touch with people after this project to get an idea their experience with ARTView.

Good points about the configuration. I think since we are aligned with PyArt, we really do our best to support the same versions and @jjhelmus has expressed that we can work together to some degree on this. I understand running older versions, I'm generally guilty of that, but with Python 3 much more mature, it seems as though we want to move (albeit slowly) toward this.

And for the record I run MacOSX 10.10.4, so this means that we do have some spread in test environments.

nguy commented 9 years ago

I just accepted PR #46 and added a couple of lines to menu.py file to hopefully make the read statement compatible with older versions of PyArt. I also changed the logic slightly so that the "PyArt does not recognize this file" warning is not displayed all of the time.

nguy commented 9 years ago

The original "issue" here seems to longer be an issue. Though it has morphed into a secondary issue.

jjhelmus commented 9 years ago

Sorry for not commenting on this and a few other issues earlier. I've been preparing a talk for and now attending the SciPy conference in Austin, TX and let the email pile up.

I expect Py-ART to support Python 2.7, 3.3, and 3.4 for quite a while. Currently the package also supports 2.6 but I'm looking to drop that support when it becomes a burden (not sure when that will be but I'll say 2.6 support is not a priority for me). The holdout for 2.6 is RHEL 5, but Nick Coghlan recently wrote a blog post arguing for dropping Python 2.6 support and offers upgrade paths to 2.7 for RHEL 5 installs.

I've worked it out so that making Py-ART releases is relatively quick and am planning on starting a month to two month release cycle for "point" (ie 1.5 vs 1.6) releases. If users install the package with conda upgrading is straightforward and a single command. All that said I think it would be reasonable if ARTView was designed to work with the latest point release of Py-ART with kludges around any functionality that only exists in the GitHub master branch.

Finally is there is an important commit to master that would warrant a quicker Py-ART release, feel free to ping me and I can either make a new release or backport the change to a minor release (ie 1.4.1).

nguy commented 9 years ago

I know you are swamped, thanks for the response @jjhelmus . I think it best to follow the same practice as Py-ART given it is a core dependency. Just let us know when you drop 2.6 and we'll do the same. And frankly I'd like to limit the amount of upkeep given this is still a side project (of love :).

I'll start another thread to discuss some future work as @gamaanderson suggested in an earlier issue.