Opened 15 years ago
Closed 15 years ago
#842 closed defect (fixed)
wx location wizard: spincontrol busted for UTM zone
Reported by: | hamish | Owned by: | |
---|---|---|---|
Priority: | critical | Milestone: | 6.4.0 |
Component: | wxGUI | Version: | svn-develbranch6 |
Keywords: | location wizard, utm | Cc: | |
CPU: | x86-64 | Platform: | Linux |
Description
Hi,
the spin control for selecting the UTM zone in the new wx Location creation wizard is busted on multiple Debian/Lennies for me but works on Michael's Mac.
For me it locks on zone "1" as soon as you try and change it.
The only way we've found to unstick it is to comment out the first place the default value of 30 is set on line 710 ( self.pval[num] = '30'
), but then I get errors in the terminal because it is uninitialized and if I continue past the resulting PROJ_INFO +lon_0 comes in at -183 degrees regardless of zone.
we tried changing the following with no luck:
- self.pval[num] = '30' + self.pval[num] = 30
- remove value=
- remove initial=
- remove style=
spin controls work ok for me in module dialogs, such as r.composite.
any ideas? anyone else see this?
also the wizard does not create UTM zones as GRASS's type PROJECTION_UTM (1) but rather as expanded +proj4 terms as PROJECTION_OTHER (99). Lat/lon correctly sets itself as PROJECTION_LL (3).
in the g.proj C code I have followed it as far as lib/proj/ convert.c's GPJ_osr_to_grass(), which has some code to specifically find the UTM zone from the +proj string, but then it trails off a bit.
Here is where it decides to use GRASS's lat/lon mode instead of go on using an "other" custom +proj string:
if (G_strcasecmp(pszToken, "proj") == 0) { /* The ll projection is known as longlat in PROJ.4 */ if (G_strcasecmp(pszValue, "longlat") == 0) pszValue = "ll"; pszProj = pszValue; continue; }
but there is no similar short-circuit for UTM zones or state plane. see also the GPJ_osr_to_grass() function header comments about return value.
State Plane (PROJECTION_SP: 2) is less of a priority for now as the wx wizard end of that hasn't been written yet, but some basic placeholder code could go in.
Hamish
Attachments (3)
Change History (24)
comment:1 by , 15 years ago
comment:3 by , 15 years ago
MIght not have any effect because it is broken on your box. We can work on this some more this week.
Michael
by , 15 years ago
Attachment: | location_wizard.patch added |
---|
Fix for UTM zone spin control and adds error trapping to extent setting page
comment:4 by , 15 years ago
I think I've fixed this. Please try the patch I just attached here. It is a diff against the svn (releasebranch_6_4) from your source tree root. The patch also adds error trapping for the default extents setting page.
Michael
comment:5 by , 15 years ago
no luck for me using the latest patch with 6.5svn. As soon as I click up or down it gets stuck at "1". If I leave it at 30 it successfully makes a zone 30 location, if it gets stuck on 1 it successfully makes a zone 1 location.
Hamish
comment:7 by , 15 years ago
Replying to cmbarton:
This patch is for 6.4, not 6.5
oh ok, it applied to 6.5 without errors just an offset of a few lines. When applied to 6.4 I get the same results. Stuck at "1".
is all the region try: rows vs. cols stuff in the patch somehow related or is that just along for the ride?
Hamish
comment:8 by , 15 years ago
I'll try one more thing. I'm pretty sure it's the binding and event handler. It has code that tries to keep the value between 1 and 60.
The other stuff is a bug fix I noticed when I was testing this. I was oversure of fixing it I guess or I wouldn't have added this fix to the patch. Have to live with it now.
Michael
follow-up: 10 comment:9 by , 15 years ago
Please try the following. Comment out lines 731-742 in the OnParamEntry method
# if self.ptype[num] == 'zone': # try: # valint = int(val) # except: # valint = 0 # if valint < 1: # self.pentry[num].SetValue(1) # val = '1' # if valint > 60: # self.pentry[num].SetValue(60) # val = '60'
This disables the validation check for the text box of the spin control. If this works, maybe I can validate this another way.
Michael
follow-up: 11 comment:10 by , 15 years ago
Replying to cmbarton:
Please try the following. Comment out lines 731-742 in the OnParamEntry method
...
This disables the validation check for the text box of the spin control. If this works, maybe I can validate this another way.
Yes- that lets me spin. If I manually type in 67 or -1 for the zone I get a pop up and it corrects me to the nearest valid value.
but if I let it spin full-scale a few times in one direction when I press continue it complains about an invalid zone even though the number I see on the screen is in-range.
Replying to cmbarton:
The other stuff is a bug fix I noticed when I was testing this. I was oversure of fixing it I guess or I wouldn't have added this fix to the patch. Have to live with it now.
probably just worry about this for the development branches for now & when you get a chance, but re. bounds settings a couple of wishes:
- popup error if s>n, or w>e (except LL), or abs(ns)>90 (LL), right now you can click the button but nothing happens if its invalid. feedback as to what the error is would be good.
- after setting the bounds the new region settings are copied to the terminal. left over debug code?
- LL bounds should accept [NnSsEeWw:] as well as [+-.0-9] chars, and res should accept ':'; currently it just accepts floats. maybe quietly accept 123d45'43.21"S format as well if we are feeling generous?
- it would be really really gee wiz cool if for LL a red rectangle would draw covering the region you've selected on the center map. even cooler but not that useful in practice would to be able to drag a box on it to get the bounds numbers close, rounded to the current resolution.
thanks, Hamish
comment:11 by , 15 years ago
Replying to hamish:
Replying to cmbarton:
Please try the following. Comment out lines 731-742 in the OnParamEntry method
...
This disables the validation check for the text box of the spin control. If this works, maybe I can validate this another way.
Yes- that lets me spin. If I manually type in 67 or -1 for the zone I get a pop up and it corrects me to the nearest valid value.
but if I let it spin full-scale a few times in one direction when I press continue it complains about an invalid zone even though the number I see on the screen is in-range.
That is REALLY weird! The SpinCtrl is limited in the code but the accompanying text box is not. My system behaves as described in the wxPython docs. I can spin all I want, but it only goes from 1->60->1->... No way to spin it above 60 or below 1. On the other hand, I can type whatever I want. If type 80, for example, the zone simply doesn't get put in. I will add a validator to the page changing handler, where it should not interfere with the spinning. But the behavior on your system is odd. Seems like a bug in the GTK version.
The error trapping I did for bounds is minimal, but it prevents the wizard from throwing an error to terminal if the rows or columns are 0. Your other suggestions are good. But like you said, should go into 6.5+.
Michael
Replying to cmbarton:
The other stuff is a bug fix I noticed when I was testing this. I was oversure of fixing it I guess or I wouldn't have added this fix to the patch. Have to live with it now.
probably just worry about this for the development branches for now & when you get a chance, but re. bounds settings a couple of wishes:
- popup error if s>n, or w>e (except LL), or abs(ns)>90 (LL), right now you can click the button but nothing happens if its invalid. feedback as to what the error is would be good.
- after setting the bounds the new region settings are copied to the terminal. left over debug code?
- LL bounds should accept [NnSsEeWw:] as well as [+-.0-9] chars, and res should accept ':'; currently it just accepts floats. maybe quietly accept 123d45'43.21"S format as well if we are feeling generous?
- it would be really really gee wiz cool if for LL a red rectangle would draw covering the region you've selected on the center map. even cooler but not that useful in practice would to be able to drag a box on it to get the bounds numbers close, rounded to the current resolution.
thanks, Hamish
by , 15 years ago
Attachment: | location_wizard.patch2 added |
---|
follow-up: 13 comment:12 by , 15 years ago
OK. I just attached a new patch for GRASS 6.4: location_wizard.patch2. It's like the first one, patched against the svn from the root of your source tree. Hopefully, this will do the trick.
Michael
follow-up: 15 comment:13 by , 15 years ago
Replying to cmbarton:
OK. I just attached a new patch for GRASS 6.4: location_wizard.patch2. It's like the first one, patched against the svn from the root of your source tree. Hopefully, this will do the trick.
no dice.
I can spin it, it loops past 60 and back to 1, but if I touch the up/down controls at all or type in a number I get a pop up window that says "Something is missing! You must enter a value for Projection Zone (1-60)". If I don't touch it at all I can go past and create a location for UTM zone 30.
after I press ok on the error popup, this appears in the terminal:
Traceback (most recent call last): File "/usr/src/grass/svn/releasebranch_6_4/dist.x86_64-unknown-linux-gnu/etc/wxpython/gui_modules/location_wizard.py", line 769, in OnPageChange if int(self.pval[num]) > 60 or int(self.pval[num]) < 1: ValueError: invalid literal for int() with base 10: ''
If I put some debug messages directly before L769,
num = self.utmzoneNum print "num=",num print "self.utmzoneNum=",self.utmzoneNum print "self.pval[num]=",self.pval[num]
I get this on the terminal after pressing ok to the error popup message:
num= 0 self.utmzoneNum= 0 self.pval[num]=
Hamish
comment:14 by , 15 years ago
Priority: | major → critical |
---|
comment:15 by , 15 years ago
Replying to hamish:
Replying to cmbarton:
OK. I just attached a new patch for GRASS 6.4: location_wizard.patch2. It's like the first one, patched against the svn from the root of your source tree. Hopefully, this will do the trick.
no dice.
I can spin it, it loops past 60 and back to 1, but if I touch the up/down controls at all or type in a number I get a pop up window that says "Something is missing! You must enter a value for Projection Zone (1-60)". If I don't touch it at all I can go past and create a location for UTM zone 30.
after I press ok on the error popup, this appears in the terminal:
Traceback (most recent call last): File "/usr/src/grass/svn/releasebranch_6_4/dist.x86_64-unknown-linux-gnu/etc/wxpython/gui_modules/location_wizard.py", line 769, in OnPageChange if int(self.pval[num]) > 60 or int(self.pval[num]) < 1: ValueError: invalid literal for int() with base 10: ''If I put some debug messages directly before L769,
num = self.utmzoneNum print "num=",num print "self.utmzoneNum=",self.utmzoneNum print "self.pval[num]=",self.pval[num]I get this on the terminal after pressing ok to the error popup message:
num= 0 self.utmzoneNum= 0 self.pval[num]=Hamish
Drat!
This is a page changing validator that someone else added since last I worked on the location wizard. Good idea, but it's wrong. I noticed but it hadn't been causing errors so I left it alone, wanting to mess with this as little as possible. I guess I have to fix it too.
Michael
comment:16 by , 15 years ago
for 6.4.0 a regular text entry box should be fine; we can keep working on the spincontrol in the dev branches and have it ready for 6.4.1.
Hamish
by , 15 years ago
Attachment: | location_wizard.patch3 added |
---|
follow-up: 18 comment:17 by , 15 years ago
I've changed the UTM Zone entry widget to a simple text widget with validation to keep it within the 1-60 range. I've added a new patch here: location_wizard.patch3. Please test.
Michael
follow-ups: 19 20 comment:18 by , 15 years ago
Replying to cmbarton:
I've changed the UTM Zone entry widget to a simple text widget with validation to keep it within the 1-60 range. I've added a new patch here: location_wizard.patch3. Please test.
Yes it works if you type in zone=1-60.
I'm seeing that +lon_0=-183 again though and I figured out how to reproduce it. If you enter "0" or "99" for the zone it automatically resets to zone 1 or 60, respectively. Apparently this is only visually. If you proceed to the summary page in the location wizard instead of +zone=23 you will see +proj=tmerc +lon_0=411 etc. ...
-PROJ_INFO------------------------------------------------- name : Transverse Mercator proj : tmerc datum : wgs84 ellps : wgs84 lat_0 : 0 lon_0 : -183 k : 0.9996 x_0 : 500000 y_0 : 0 no_defs : defined -PROJ_UNITS------------------------------------------------ unit : Meter units : Meters meters : 1
fwiw1: I notice that PROJ_UNITS is uppercased, which seems different to how I always seem to have remembered it there.
fwiw2: I notice this patch no longer has the try: row/column stuff. (different things should go into 2 different commits I guess)
Hamish
comment:19 by , 15 years ago
Replying to hamish:
fwiw1: I notice that PROJ_UNITS is uppercased, which seems different to how I always seem to have remembered it there.
this may be problematic, I see that lib/gis/proj2.c and proj3.c do work pretty much raw from the PROJ_UNITS file. there is a lower() function in there, but I am not confident that it covers all bases. Probably want a double fix at both ends if possible.
Hamish
comment:20 by , 15 years ago
Replying to hamish:
Replying to cmbarton:
I've changed the UTM Zone entry widget to a simple text widget with validation to keep it within the 1-60 range. I've added a new patch here: location_wizard.patch3. Please test.
Yes it works if you type in zone=1-60.
I'm seeing that +lon_0=-183 again though and I figured out how to reproduce it. If you enter "0" or "99" for the zone it automatically resets to zone 1 or 60, respectively. Apparently this is only visually. If you proceed to the summary page in the location wizard instead of +zone=23 you will see +proj=tmerc +lon_0=411 etc. ...
-PROJ_INFO------------------------------------------------- name : Transverse Mercator proj : tmerc datum : wgs84 ellps : wgs84 lat_0 : 0 lon_0 : -183 k : 0.9996 x_0 : 500000 y_0 : 0 no_defs : defined -PROJ_UNITS------------------------------------------------ unit : Meter units : Meters meters : 1
This is all fixed and now committed. I'll try to port this to to 6.5 and 7 after it's had a few more tests--and will then mark this resolved. Thanks for all your help in getting this worked out. The bindings and events generated by the spinctrl were byzantine. This works much better.
fwiw1: I notice that PROJ_UNITS is uppercased, which seems different to how I always seem to have remembered it there.
I don't understand what you're referring to. There is no upper() function in the module and nothing named proj_units that I could find. The location wizard does not set the proj units. This is done by g.proj when the proj4 parameters are sent to it. I use the lower() function to make searching more rewarding regardless of case.
fwiw2: I notice this patch no longer has the try: row/column stuff. (different things should go into 2 different commits I guess)
I had just archived this aside until I got the spin box fixed. It is in the bugfix commit.
Michael
Hamish
comment:21 by , 15 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
ah, I've just spotted this "hack around some weirdness", which may be a major clue..
source:grass/branches/develbranch_6/gui/wxpython/gui_modules/location_wizard.py@40163#L720
Hamish