search

nguy / artview

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

Topography Map 2 #159

Closed gamaanderson closed 8 years ago

gamaanderson commented 8 years ago

as the other PR turned into a discussion and I don't want to interrupt it, I'm resubmiting it.

This includes a image background and a warning before loading the topography from the internet.

nguy commented 8 years ago

@gamaanderson This is a nice improvement over the previous PR. I'm actually a huge fan of the background image. Some neat possibilities here.

The code itself looks solid. I think we should merge this in for now. But maybe we should add a warning about Basemap being mothballed in 4 years? As much a reminder for us as for the user.

gamaanderson commented 8 years ago

about the warning, we have 4 years, that is kind of a lot and we will not forget since pyart will make the transition at some time.

I'm ok with merging this, but with one comment, I don't think this background is usable as an every day feature, the method of adding it is too complicated (too many clicks). This is probably only usable as an eventual feature while doing a image for instance. So I don't really think this will be much used, but it stay as a demonstration of potential if at some time our users decide they want/need it.

This also open the question of defining our public, for instance: for the everyday weather vigilance a background (political or topographic) is fundamental. As much as I would like to address this group, we must focus in your actual (current) users. I kind of depend on you, that has much more experience and contact, to be a compass in this aspect.

nguy commented 8 years ago

Fair enough on the warning.

I agree with you about the image. I'm not sure it is highly usable and may cause confusion. Can we remove the Image mode (comment out) and reinstall later as it becomes clear it may be needed?

nguy commented 8 years ago

Per your point about our users, I don't think ARTview is currently suited for real-time weather analysis (yet?). I think it best is used in analysis and post data collection browsing. This was the intent from the beginning. Having a topographical or boundary map can be fundamental in the analysis of some data, so I think this is a good idea.

gamaanderson commented 8 years ago

to remove the mode, just comment it out in the modes list of dicts. It still acessible as a pluging using manual linking.

nguy commented 8 years ago

I think that's a good way to proceed. I'll merge this for now. Pull in the other awaiting PR and then submit a really minor PR with this change and any other minor things I find in the code base.

nguy commented 8 years ago

PR #161 comments out this mode