sharppy / SHARPpy

Sounding/Hodograph Analysis and Research Program in Python
https://sharppy.github.io/SHARPpy/index.html
Other
221 stars 110 forks source link

Add Surface Temperature To Winter Panel #80

Closed pmarshwx closed 8 years ago

pmarshwx commented 8 years ago

Here's a quick fix to add surface temperature to the winter panel. This was a frequent request from SPC forecasters who utilize this functionality in the SPC's version of SHARP.

Also, my text editor strips trailing white space and spaces in blank lines, so many lines may have been modified here.

keltonhalbert commented 8 years ago

Hi Patrick,

I've given it a test, and it looks like the text you've added isn't rendering inside the window. I'm on Mac OS X Yosemite right now, so I'll give it a run on the linux box, which as a bigger display, later today to make sure. test

keltonhalbert commented 8 years ago

Aaaannnnd I've got a bad case of the Mondays. I was testing the wrong branch.

keltonhalbert commented 8 years ago

Alright, now I've got the right version.
It does look like some sizing issues are going on on my 13in MBP, so there are two options... 1: We need to implement the autosizing text as in some of the other insets 2: We can bring the bars up in the warm/cold layers output test

pmarshwx commented 8 years ago

OK. It works on a mbp15, so I never saw this issue.

What if we made one of those boxes scrollable? Maybe have the warm.cold layers scroll if they go too far? Unfortunately, I'm not sure how best to implement that in Qt, so I'd need to work with someone on this.

keltonhalbert commented 8 years ago

As far as making the warm/cold layers scrollable, I think that's a great idea but I would also have to do some digging into how that would work. Right now, the widget that makes up the Winter Inset is just a QWidget with a QPixMap displayed - all drawing happens on the pixmap and then gets rendered on show or update. Therefore, it might involve nesting QWidgets (which is possible) inside of the inset and defining some containers to allow scrolling. In theory, doable - not sure exactly how it would look in practice. I'd be willing to work with you on this since I've done a large chunk of the GUI if you think that the scrolling is the way to go.

pmarshwx commented 8 years ago

I'm certainly interested in learning more about the GUI, but I'm by no means convinced this is the "correct" solution. I just know that we should come up with something soon as knowing the surface temperature is crucial in winter forecasting (with precision to no decimal places you don't now what rounding is going on). This is why SPC was asking for it.

keltonhalbert commented 8 years ago

I think we can make it squeeze without doing scrolling for now. Perhaps something we can try in the near future. I agree, timelines is important here.

You'll have to forgive my noobishness, but how do I edit the code in your pull request before merging it? I'm just going to modify the overall height of the inset data.

pmarshwx commented 8 years ago

I think You need to fork my branch and submit a request to my branch.

pmarshwx commented 8 years ago

@dopplershift may be able to provide additional insight on how to do it.

dopplershift commented 8 years ago

@pmarshwx's suggestion is right, except you just check out his branch, not fork--this works because his branch is off a clone of SharpPy. The other option (probably not necessary in this case) is to check out his branch and submit a new PR.

To grab his branch:

git remote add pmarshwx git@github.com:pmarshwx/SHARPpy
git fetch pmarshwx
git checkout -b winter-fix pmarshwx/winter-fix

Commit fix, then push to github, and submit as a PR against @pmarshwx's fork and branch.

pmarshwx commented 8 years ago

Thanks, Ryan. That's more-or-less what I remembered, minus the checkout (not fork).

keltonhalbert commented 8 years ago

Thanks! I've got a couple of flights later this evening. I'll get the size adjusted and submit it to your winter-fix branch. Once you confirm that it still looks OK on your end, I think we can merge this pull request.

pmarshwx commented 8 years ago

Given the prospects of a strong storm later this week, any thoughts on this? Do you want me to resize and push again?

tsupinie commented 8 years ago

Instead of putting the surface temperature below the precipitation type, would it be possible to put it on the same line? There's a ton of unused space there. On Dec 20, 2015 7:34 PM, "Patrick Marsh" notifications@github.com wrote:

Given the prospects of a strong storm later this week, any thoughts on this? Do you want me to resize and push again?

— Reply to this email directly or view it on GitHub https://github.com/sharppy/SHARPpy/pull/80#issuecomment-166171649.

tsupinie commented 8 years ago

Okay, I've done the procedure that @dopplershift outlined and submitted a pull request to @pmarshwx's branch. Essentially, I just reduced the vertical spacing to cram everything in there some more.

pmarshwx commented 8 years ago

@tsupinie's code works on my laptop. Now for someone with a 13" laptop to test.

wblumberg commented 8 years ago

@keltonhalbert ?

keltonhalbert commented 8 years ago

It works! winter