Opened 7 years ago

Closed 5 years ago

#3502 closed defect (fixed)

v.proj location not set in dialog

Reported by: balagates Owned by: grass-dev@…
Priority: normal Milestone: 7.8.3
Component: wxGUI Version: git-releasebranch78
Keywords: v.proj location Cc:
CPU: OSX/Intel Platform: MacOSX

Description

In the Source tab for the v.proj dialog are three entry fields - location, mapset and name. The pulldown list for location seems appropriate and you can select an item, however, this does not update the dialog. You can tell because 1) the status dialog at the bottom still shows "location=<required>" and the subsequent entry field pulldown lists are not populated as you would expect. A workaround is to type the location name in manually in the text box. Then everything works as expected.

The other pulldowns, mapset and name, in this dialog seems to work properly and update the dialog when an element in the pulldown is selected.

This is a new behavior for me in 7.4.0. It worked as I expected in 7.2.2.

Attachments (1)

wx_combo_events.py (1.5 KB ) - added by balagates 6 years ago.
wx.ComboBox behavior test program

Download all attachments as: .zip

Change History (12)

comment:1 by martinl, 7 years ago

Milestone: 7.2.4

comment:2 by balagates, 7 years ago

I believe I found what caused this change. In ./gui/wxpython/gui_core/forms.py 7.2.2 (71322) vs 7.4.0 (71217)

                        elif prompt == 'location':
                            win = gselect.LocationSelect(parent=which_panel,
                                                         value=value)
                            win.Bind(wx.EVT_COMBOBOX, self.OnUpdateSelection)
                            win.Bind(wx.EVT_COMBOBOX, self.OnSetValue)

changed to

                        elif prompt == 'location':
                            win = gselect.LocationSelect(parent=which_panel,
                                                         value=value)
                            win.Bind(wx.EVT_TEXT, self.OnUpdateSelection)
                            win.Bind(wx.EVT_TEXT, self.OnSetValue)

There were a large number of other changes that appear unimportant for this. I do not understand why the 7.2.2 is a higher revision than the 7.4.0 but it must be due to different branches.

I am not confident enough to make a change to the source. Is there some other case where the change made sense? Which branch is the right one to make the change in?

After changing to EVT_COMBOBOX the selection from the pulldown list appears to work as I expect but then typing in the location no longer appears to work. In my use the former seems more important but there might be cases where the later is more important.

comment:3 by balagates, 6 years ago

I now have access to Grass 7.4.0 with wxPython 3.0.2.0 on Ubuntu 18.04. In this version v.proj location works as expected. Therefore, this problem is likely due to some difference in OS (macOS vs Linux), likely in the wxPython implementation. I verified the source on Ubuntu is win.Bind(wx.EVT_TEXT,...). The older code still makes more sense to me. Not sure if there is some benefit to the newer change.

in reply to:  3 comment:4 by hellik, 6 years ago

Replying to balagates:

I now have access to Grass 7.4.0 with wxPython 3.0.2.0 on Ubuntu 18.04. In this version v.proj location works as expected. Therefore, this problem is likely due to some difference in OS (macOS vs Linux), likely in the wxPython implementation. I verified the source on Ubuntu is win.Bind(wx.EVT_TEXT,...). The older code still makes more sense to me. Not sure if there is some benefit to the newer change.

there is now GRASS 7.6 released; any chance to test it there?

comment:5 by balagates, 6 years ago

On my Mac running macOS 10.12.6 I installed the pre-built Grass 7.6.0 app linked on the Grass download page. Within the Grass app it reports Python 2.7.15 and wxPython 4.0.3. The GUI for v.proj still seems to have the same problem and maybe an additional new problem. From the main GUI menu I select "File -> Import Vector Data -> Reproject vector map from different GRASS location [v.proj]". A window appears with the Source tab selected and 3 combo boxes (Location, Mapset, Name). The pull-down for the Location ComboBox is populated with values I expect. Selecting one from the pull-down does not set the location. At the bottom of the windows is the assembled command line and after making a selection from the pull-down is says "v.proj location=<required>". If instead of selecting the value from the ComboBox pulldown I type in location (or do some manual edits on the one I pulled down, the location gets set.

So my guess is when selecting a ComboBox on macOS wxPython is generating a wx.EVT_COMBOBOX event but not a wx.EVT_TEXT event. Since Grass 7.2 there have been several changes in forms.py changing bindings from wx.EVT_COMBOBOX to wx.EVT_TEXT with resulting loss of functionality on macOS. Since it appears to work on my Ubuntu machine the events generated for a ComboBox must be different. I haven't studied wxPython to see what events we expect when a ComboBox selection is made. To me it does make more sense that when making a ComboBox selection a wx.EVT_COMBOBOX would be generated. It is not so obvious that a wx.EVT_TEXT would get generated...but maybe it will? I do not know enough now to know what should be robust.

Note also that some changes in Grass 7.4.3 forms.py appear to have caused similar problems with the Mapset ComboBox selection. I am still studying that but the symptoms are the same. Selecting a value from the ComboBox pulldown does nothing, but typing in a Mapset name in the field sets it values and populates the Name ComboBox.

by balagates, 6 years ago

Attachment: wx_combo_events.py added

wx.ComboBox behavior test program

comment:6 by balagates, 6 years ago

I created a small test program in Python to examine wx.ComboBox() events (attached). This confirms my suspicions that on my macOS system changing the value of the ComboBox value through the pulldown menu only generates a wx.EVT_COMBOBOX event. It does not generate a wx.EVT_TEXT event. On my Ubuntu system changing the value of the ComboBox value through the pulldown menu generates both a wx.EVT_COMBOBOX event and wx.EVT_TEXT event. When the text is manually edited by typing both systems generate a wx.EVT_TEXT for each keypress.

The wxPython documentation at https://docs.wxpython.org/wx.ComboBox.html seems inconclusive about what events should be generated for each action.

  • EVT_COMBOBOX: Process a wxEVT_COMBOBOX event, when an item on the list is selected. Note that calling GetValue returns the new value of selection.
  • EVT_TEXT: Process a wxEVT_TEXT event, when the combobox text changes.

I could interpret this both ways. The fundamental question is whether these two events should be interpreted as:

  • EVT_COMBOBOX_CHANGE and EVT_TEXT_EDIT_CHANGE

or

  • EVT_COMBOBOX_CHANGE and EVT_VALUE_CHANGE

It is unclear if the wx intention was to generate a 1:1 mapping between change method and event. Neither system generates any events when you use the ComboBox pulldown and select the element that is already selected.

It would be interesting to see how this behaves on a Windows system, however, majority behavior probably doesn't imply correctness.

In the Grass v.proj case I cannot see a common use for manually editing the values. Sure, it might be quicker for someone with a long list of similarly named elements to manually edit, but I think the idea of populating the ComboBox is to avoid typing mistakes.

Potentially this could affect the Grass Python GUI other places so it probably deserves more investigation.

comment:7 by balagates, 6 years ago

I did a little searching in wxWidgets and wxPython to try understand the intentions of the wx.ComboBox events. The only thing I found somewhat useful were comments in https://github.com/wxWidgets/wxPython/blob/master/demo/ComboBox.py that said

  • wx.EVT_COMBOBOX - When the user selects something.
  • wx.EVT_TEXT - Capture events every time the user hits a key in the text entry field.
  • wx.EVT_TEXT_ENTER - Capture events when the user types something into the control then hits ENTER.

This functionality seems to match what I see on macOS. The ComboBox implementation in wxWidgets for Linux GTK is complicated and appears to have compile or configuration options that affect how events are handled. I had difficulty following so I could not explain the behavior on Linux.

For robustness I do not think I would depend solely on wx.EVT_TEXT messages being generated when a user selects a new value through the pulldown.

comment:8 by sbl, 6 years ago

Milestone: 7.2.47.4.5

comment:9 by nila, 5 years ago

This issue is still valid for master (GRASS 7.9dev), with wxPython version: 4.0.7.post2 osx-cocoa (phoenix) and wxWidgets 3.0.5.

A PR with a fix has been made: https://github.com/OSGeo/grass/pull/363.

Thanks @balagates for a great bug investigation!

comment:10 by neteler, 5 years ago

Milestone: 7.4.57.8.3
Version: 7.4.0git-releasebranch78

comment:11 by annakrat, 5 years ago

Resolution: fixed
Status: newclosed

Fixed and merged to 7.8, thank you!

Note: See TracTickets for help on using tickets.