wxGUI/mapwin: add map overlays 'at' parameter arg validation by tmszi · Pull Request #1069 · OSGeo/grass

tmszi · 2020-11-05T10:42:35Z

To Reproduce:

Steps to reproduce the behavior:

Launch wxGUI g.gui
Display elevation raster map
Add elevation raster map legend
On the Map display window double click on the added elevation raster map legend
On the d.legend dialog go to Optional tab (page)
Set 'at' parameter argument (which required four number at=bottom,top,left,right) but set only one number e.g. 5
Hit Apply or OK button

Error message:

ERROR: Failed to run command 'd.legend raster=elevation at=5'. Details:

       ERROR: Option <at> must be provided in multiples of 4
       You provided 1 item(s): 5
Traceback (most recent call last):
  File "/usr/lib64/grass79/gui/wxpython/core/gthread.py", line 137, in OnDone
    event.ondone(event)
  File "/usr/lib64/grass79/gui/wxpython/core/render.py", line 483, in OnRenderDone
    self.updateProgress.emit(env=event.userdata['env'], layer=self.layer)
  File "/usr/lib64/grass79/etc/python/grass/pydispatch/signal.py", line 230, in emit
    dispatcher.send(signal=self, *args, **kwargs)
  File "/usr/lib64/grass79/etc/python/grass/pydispatch/dispatcher.py", line 350, in send
    **named
  File "/usr/lib64/grass79/etc/python/grass/pydispatch/robustapply.py", line 60, in robustApply
    return receiver(*arguments, **named)
  File "/usr/lib64/grass79/gui/wxpython/core/render.py", line 774, in ReportProgress
    self.renderDone.emit(env=env)
  File "/usr/lib64/grass79/etc/python/grass/pydispatch/signal.py", line 230, in emit
    dispatcher.send(signal=self, *args, **kwargs)
  File "/usr/lib64/grass79/etc/python/grass/pydispatch/dispatcher.py", line 350, in send
    **named
  File "/usr/lib64/grass79/etc/python/grass/pydispatch/robustapply.py", line 60, in robustApply
    return receiver(*arguments, **named)
  File "/usr/lib64/grass79/gui/wxpython/mapwin/buffered.py", line 967, in _updateMFinished
    coords=self.overlays[id].coords)
  File "/usr/lib64/grass79/gui/wxpython/mapwin/decorations.py", line 105, in GetCoords
    (self._renderer.width, self._renderer.height))
  File "/usr/lib64/grass79/gui/wxpython/mapwin/decorations.py", line 304, in GetPlacement
    '=')[1].split(',')]  # pylint: disable-msg=W0612
ValueError: not enough values to unpack (expected 4, got 1)

Expected behavior:

petrasovaa

This solution is very specific to d.legend (and some other d.* modules), I would rather go with the system of validators used in forms.py, that is already in place, see for example CoordinatesValidator as the closest example. The disadvantage is that with the current validators in forms you don't get any information what is wrong and I don't think it prevents you from trying to run the module. The widget background turns grey, that's all it does.

However, I would rather improve the system of validators in general than going with this too specialized solution. For this PR I would try to create a new validator (maybe generalize the coordinates validator, based on option's 'key_desc'). For future I would like to see better reporting of the problems, e.g. through an InfoBar in each module frame. The validator could call a callback with the message and the module form would show the infobar.

tmszi · 2021-01-09T06:15:04Z

Fixed in the cac364b.

Also removed focus when invalid since it selects all text in the textctrl and you can't keep typing

petrasovaa · 2021-01-09T19:46:28Z

It didn't work for me, so I made some changes, see the commit message. If this works for you, feel free to merge it.

tmszi · 2021-01-09T20:08:40Z

It didn't work for me, so I made some changes, see the commit message. If this works for you, feel free to merge it.

Yes it work I tested many time before I send my commits.

According wxPython doc Validate method has parent arg https://wxpython.org/Phoenix/docs/html/wx.Validator.html, this line is no needed.

petrasovaa · 2021-01-10T03:03:07Z

It didn't work for me, so I made some changes, see the commit message. If this works for you, feel free to merge it.

Yes it work I tested many time before I send my commits.

According wxPython doc Validate method has parent arg https://wxpython.org/Phoenix/docs/html/wx.Validator.html, this line is no needed.

Weird, this is what I was getting:

Traceback (most recent call last):
  File "/home/anna/dev/grass/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/gui_core/widgets.py", line 586, in OnText
    self.Validate()
TypeError: Validate() missing 1 required positional argument: 'win'

Any idea what could be going on here?

tmszi · 2021-01-10T07:58:56Z

It didn't work for me, so I made some changes, see the commit message. If this works for you, feel free to merge it.

Yes it work I tested many time before I send my commits.
According wxPython doc Validate method has parent arg https://wxpython.org/Phoenix/docs/html/wx.Validator.html, this line is no needed.

Weird, this is what I was getting:
Traceback (most recent call last):
  File "/home/anna/dev/grass/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/gui_core/widgets.py", line 586, in OnText
    self.Validate()
TypeError: Validate() missing 1 required positional argument: 'win'
Any idea what could be going on here?

Base class BaseValidator Validate method has missing parent (win) argument, commit 9515755.

petrasovaa · 2021-01-11T15:38:22Z

Any idea what could be going on here?

Base class BaseValidator Validate method has missing parent (win) argument, commit 9515755.

I didn't test it but shouldn't the classes derived from BaseValidator be updated as well?

tmszi · 2021-01-11T17:40:31Z

Any idea what could be going on here?

Base class BaseValidator Validate method has missing parent (win) argument, commit 9515755.

I didn't test it but shouldn't the classes derived from BaseValidator be updated as well?

Yes of course you're right, I did it in the commit above.

)

tmszi added bug Something isn't working GUI wxGUI related labels Nov 5, 2020

petrasovaa requested changes Nov 30, 2020

View reviewed changes

wxGUI/forms: add map overlays 'at' arg parameter validation

cac364b

tmszi force-pushed the fix-validate-map-overlays-at-arg branch from f6db125 to cac364b Compare January 9, 2021 06:12

tmszi requested a review from petrasovaa January 9, 2021 06:15

tmszi and others added 2 commits January 9, 2021 14:27

fix PlacementValidator class Validate method win arg

1d1e814

fix validation and call btn.Enable with T/F parameter

fc2be51

Also removed focus when invalid since it selects all text in the textctrl and you can't keep typing

petrasovaa approved these changes Jan 9, 2021

View reviewed changes

tmszi added 2 commits January 10, 2021 08:40

fix BaseValidator class Validate method win arg

9515755

fix method doc arg type

a76d5cd

petrasovaa mentioned this pull request Jan 26, 2021

[Bug] Error message on reprojection in Data tab #1273

Closed

petrasovaa merged commit 186b98a into OSGeo:master Feb 17, 2021

petrasovaa added the backport_needed label Feb 17, 2021

tmszi deleted the fix-validate-map-overlays-at-arg branch February 17, 2021 21:25

petrasovaa removed the backport_needed label Mar 6, 2021

petrasovaa added a commit to petrasovaa/grass that referenced this pull request Mar 6, 2021

wxGUI: fix validators (partial backport of OSGeo#1069 and OSGeo#1418)

3e51ad2

petrasovaa added a commit that referenced this pull request Mar 6, 2021

wxGUI: fix validators (partial backport of #1069 and #1418) (#1427)

18803e7

marisn pushed a commit to marisn/grass that referenced this pull request Mar 22, 2021

wxGUI/mapwin: add map overlays 'at' parameter arg validation (OSGeo#1069

353899f

)

neteler added this to the 7.8.5 milestone Dec 9, 2021

ninsbl pushed a commit to ninsbl/grass that referenced this pull request Oct 26, 2022

wxGUI/mapwin: add map overlays 'at' parameter arg validation (OSGeo#1069

b36956a

)

ninsbl pushed a commit to ninsbl/grass that referenced this pull request Feb 17, 2023

wxGUI/mapwin: add map overlays 'at' parameter arg validation (OSGeo#1069

4601360

)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

wxGUI/mapwin: add map overlays 'at' parameter arg validation#1069

wxGUI/mapwin: add map overlays 'at' parameter arg validation#1069
petrasovaa merged 5 commits intoOSGeo:masterfrom
tmszi:fix-validate-map-overlays-at-arg

tmszi commented Nov 5, 2020

Uh oh!

petrasovaa left a comment

Uh oh!

tmszi commented Jan 9, 2021

Uh oh!

petrasovaa commented Jan 9, 2021

Uh oh!

tmszi commented Jan 9, 2021

Uh oh!

petrasovaa commented Jan 10, 2021

Uh oh!

tmszi commented Jan 10, 2021

Uh oh!

petrasovaa commented Jan 11, 2021

Uh oh!

tmszi commented Jan 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Comments

Conversation

tmszi commented Nov 5, 2020

Uh oh!

petrasovaa left a comment

Choose a reason for hiding this comment

Uh oh!

tmszi commented Jan 9, 2021

Uh oh!

petrasovaa commented Jan 9, 2021

Uh oh!

tmszi commented Jan 9, 2021

Uh oh!

petrasovaa commented Jan 10, 2021

Uh oh!

tmszi commented Jan 10, 2021

Uh oh!

petrasovaa commented Jan 11, 2021

Uh oh!

tmszi commented Jan 11, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants