Skip to content

Comments

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

Merged
tmszi merged 2 commits intoOSGeo:mainfrom
tmszi:wxgui-fix-save-web-service-layer
Aug 1, 2023
Merged

wxGUI/lmgr: fix saving web service layer as raster map#3101
tmszi merged 2 commits intoOSGeo:mainfrom
tmszi:wxgui-fix-save-web-service-layer

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 labels Jul 26, 2023
@tmszi tmszi added this to the 8.4.0 milestone Jul 26, 2023
wenzeslaus
wenzeslaus requested changes Jul 27, 2023
View reviewed changes
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

petrasovaa commented Jul 27, 2023

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

wenzeslaus
wenzeslaus approved these changes Jul 28, 2023
View reviewed changes
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
tmszi added a commit to tmszi/grass that referenced this pull request Aug 1, 2023
@tmszi @petrasovaa
wxGUI/lmgr: fix saving web service layer as raster map (OSGeo#3101)
52574bf 
Co-authored-by: Anna Petrasova <[email protected]>
tmszi added a commit to tmszi/grass that referenced this pull request Aug 1, 2023
@tmszi @petrasovaa
wxGUI/lmgr: fix saving web service layer as raster map (OSGeo#3101)
33b3b1c 
Co-authored-by: Anna Petrasova <[email protected]>
@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
@tmszi @petrasovaa @landam
wxGUI/lmgr: fix saving web service layer as raster map (OSGeo#3101)
13f98d3 
Co-authored-by: Anna Petrasova <[email protected]>
neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023
@tmszi @petrasovaa @neteler
wxGUI/lmgr: fix saving web service layer as raster map (OSGeo#3101)
703d556 
Co-authored-by: Anna Petrasova <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wenzeslaus wenzeslaus wenzeslaus approved these changes

Assignees

No one assigned

Labels

bug Something isn't working GUI wxGUI related

Projects

None yet

Milestone

8.4.0

Development

Successfully merging this pull request may close these issues.

3 participants

@tmszi @petrasovaa @wenzeslaus