Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wxGUI/lmgr: fix saving web service layer as raster map #3101

Merged
merged 2 commits into from
Aug 1, 2023

Conversation

tmszi
Copy link
Member

@tmszi tmszi commented Jul 26, 2023

Describe the bug
Saving web service layer as raster map fail.

To Reproduce
Steps to reproduce the behavior:

  1. From the GUI Layers pane toolbar launch Add web service map dialog
  2. Hit Add default button
  3. Choose OSM-WMS item from the load profiles ComboBox widget
  4. Hit Connect button
  5. From the tree list of layers choose some layer e.g. OpenStreetMap WMS - by terrestris
  6. Hit Add layer button
  7. From the Layers pane choose added web service layer OpenStreetMap WMS - by terrestris and by right mouse click invoke layer menu and choose Save web service layer menu item
  8. See error
Traceback (most recent call last):
  File "/usr/lib64/grass84/gui/wxpython/lmgr/layertree.py",
line 862, in OnSaveWs

parent=self, layer=mapLayer, giface=self._gifaceForDisplay
AttributeError
:
'LayerTree' object has no attribute '_gifaceForDisplay'

Expected behavior
Saving web service layer as raster map should work.

System description (please complete the following information):

  • Operating System: all
  • GRASS GIS version: 8.4.0dev, 8.3, 8.2

@tmszi tmszi added bug Something isn't working GUI wxGUI related backport to 8.3 PR needs to be backported to release branch 8.3 backport to 8.2 PR needs to be backported to release branch 8.2 labels Jul 26, 2023
@tmszi tmszi added this to the 8.4.0 milestone Jul 26, 2023
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks wrong. It is accessing a private attribute of self.mapdisplay. The "GRASS interface" references are meant to be shared to objects you create when appropriate, but not the other way around. self._giface like on line 871 would be how the code usually looks like.

@tmszi
Copy link
Member Author

tmszi commented Jul 27, 2023

This looks wrong. It is accessing a private attribute of self.mapdisplay. The "GRASS interface" references are meant to be shared to objects you create when appropriate, but not the other way around. self._giface like on line 871 would be how the code usually looks like.

Yes I know about it. Original arg self._gifaceForDisplay point to LayerManagerGrassInterfaceForMapDisplay class instance which is not avilable as private atribute for LayerTree class instance. self._giface atribute point to LayerManagerGrassInterface class instance.

Should I initialize a new LayerManagerGrassInterfaceForMapDisplay class instance or use existed self._giface atrib as save web service layer class giface param arg?

@petrasovaa
Copy link
Contributor

This is probably a more correct solution, but I did just brief testing.

Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The callback is actually a factory method so in a sense it is similar to constructor. In the context of giface concept, constructors should generally get the right giface rather than try to create it themselves. This moves creation of giface out of the callback and adds giface as its parameter, so it looks like the right solution. It also removes the duplicate code between the two callbacks which is nice and usually a good sign.

@tmszi tmszi merged commit f518403 into OSGeo:main Aug 1, 2023
19 checks passed
tmszi added a commit to tmszi/grass that referenced this pull request Aug 1, 2023
Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
tmszi added a commit to tmszi/grass that referenced this pull request Aug 1, 2023
Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
@tmszi tmszi removed backport to 8.3 PR needs to be backported to release branch 8.3 backport to 8.2 PR needs to be backported to release branch 8.2 labels Aug 1, 2023
@tmszi tmszi deleted the wxgui-fix-save-web-service-layer branch August 1, 2023 03:17
landam pushed a commit to landam/grass that referenced this pull request Oct 25, 2023
Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
Co-authored-by: Anna Petrasova <kratochanna@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working GUI wxGUI related
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants