[GRASS-dev] [GRASS GIS] #842: wx location wizard: spincontrol busted for UTM zone

#842: wx location wizard: spincontrol busted for UTM zone
----------------------------------+-----------------------------------------
Reporter: hamish | Owner: grass-dev@lists.osgeo.org
     Type: defect | Status: new
Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Keywords: location wizard, utm | Platform: Linux
      Cpu: x86-64 |
----------------------------------+-----------------------------------------
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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

ah, I've just spotted this "hack around some weirdness", which may be a
major clue..

{{{
# deal with weird spin control text event behavior, sending dup. event
with negative number
    if event.GetId() >= 2000:
       num = event.GetId() - 2000
}}}

source:grass/branches/develbranch_6/gui/wxpython/gui_modules/location_wizard.py@40163#L720

Hamish

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

... commenting that out has no effect

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by cmbarton):

MIght not have any effect because it is broken on your box. We can work on
this some more this week.

Michael

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by cmbarton):

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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/842#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by cmbarton):

This patch is for 6.4, not 6.5

Michael

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:6&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

Replying to [comment:6 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:7&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by cmbarton):

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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:8&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by cmbarton):

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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:9&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

Replying to [comment:9 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 [comment:8 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:10&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by cmbarton):

Replying to [comment:10 hamish]:
> Replying to [comment:9 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 [comment:8 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:11&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by 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.

Michael

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:12&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
---------------------+------------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: major | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
---------------------+------------------------------------------------------
Comment (by hamish):

Replying to [comment:12 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:13&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
-----------------------+----------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Changes (by hamish):

  * priority: major => critical

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:14&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
-----------------------+----------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by cmbarton):

Replying to [comment:13 hamish]:
> Replying to [comment:12 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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:15&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
-----------------------+----------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by hamish):

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

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/842#comment:16&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
-----------------------+----------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by 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.

Michael

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/842#comment:17&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
-----------------------+----------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by hamish):

Replying to [comment:17 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/842#comment:18&gt;
GRASS GIS <http://grass.osgeo.org>

#842: wx location wizard: spincontrol busted for UTM zone
-----------------------+----------------------------------------------------
  Reporter: hamish | Owner: grass-dev@lists.osgeo.org
      Type: defect | Status: new
  Priority: critical | Milestone: 6.4.0
Component: wxGUI | Version: svn-develbranch6
Resolution: | Keywords: location wizard, utm
  Platform: Linux | Cpu: x86-64
-----------------------+----------------------------------------------------
Comment (by hamish):

Replying to [comment:18 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

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/842#comment:19&gt;
GRASS GIS <http://grass.osgeo.org>