wxGUI/dbmgr: fix removing map table layer and layer related Browse data and Manage tables page (tab) by tmszi · Pull Request #2422 · OSGeo/grass

tmszi · 2022-06-09T14:42:33Z

Describe the bug
Removing map table layer don't work and related layer page (tab) on the Browse data and Manage tables page (tab) aren't deleted too.

To Reproduce
Steps to reproduce the behavior:

Launch Attribute Table Manager e.g g.gui.dbmgr geology
Switch to Manage layers page (tab)
Create new table with name e.g. test (right side on the Add layer page (tab))
Add new layer with layer number 2, and choose test table from table choice widget and hit Add layer button
On the Manage layers page (tab) switch to Remove layer page (tab)
Choose layer 2 from Layer to remove ComboBox widget choices and hit Remove layer button
See error

Traceback (most recent call last):
  File "/usr/lib64/grass83/gui/wxpython/dbmgr/base.py", line 3815, in OnDeleteLayer
    self.parentDialog.parentDbMgrBase.UpdateDialog(layer=layer)
  File "/usr/lib64/grass83/gui/wxpython/dbmgr/manager.py", line 253, in UpdateDialog
    DbMgrBase.UpdateDialog(self, layer=layer)
  File "/usr/lib64/grass83/gui/wxpython/dbmgr/base.py", line 860, in UpdateDialog
    self.pages["browse"].DeletePage(layer)
  File "/usr/lib64/grass83/gui/wxpython/dbmgr/base.py", line 1037, in DeletePage
    self.selLayer = self.layers[self.GetSelection()]
IndexError: list index out of range

Expected behavior
Removing map table layer should be work without error message, and related layer page (tab) on the Browse data and Manage tables page (tab) be deleted too.

System description (please complete the following information):

Operating System: all
GRASS GIS version: all

petrasovaa · 2022-06-10T03:42:03Z

gui/wxpython/dbmgr/base.py

            return False

-        GNotebook.DeleteNBPage(self, self.layers.index(layer))
+        GNotebook.DeletePage(self, self.layers.index(layer))


Could you please comment on this more? The new code uses directly the method of the FlatNotebook, but why? Is something wrong about GNotebook's implementation? Wouldn't it be possibly better to adjust the code of this function to work with the GNotebook's version of DeletNBPage?

NotebookController base class, InsertPage method has 2 way how to insert page:

grass/gui/wxpython/gui_core/widgets.py

Lines 143 to 154 in 120f198

def InsertPage(self, *args, **kwargs):

"""Insert a new page"""

if "name" in kwargs:

self.notebookPages[kwargs["name"]] = kwargs["page"]

del kwargs["name"]

try:

self.classObject.InsertPage(self.widget, *args, **kwargs)

except TypeError as e: # documentation says 'index', but certain versions of wx require 'n'

kwargs["n"] = kwargs["index"]

del kwargs["index"]

self.classObject.InsertPage(self.widget, *args, **kwargs)

Insert page with name param

Insert page without name param (this is preferred way in Attribute Table Manager code)

grass/gui/wxpython/dbmgr/base.py

Lines 1173 to 1178 in 120f198

self.InsertNBPage(

index=pos,

page=panel,

text=" %d / %s %s"

% (layer, label, self.dbMgrData["mapDBInfo"].layers[layer]["table"]),

)

NotebookController base class, inside DeletePage method is called method GetPageIndexByName which is not return page index (return -1) if page was inserted without name param.

grass/gui/wxpython/gui_core/widgets.py

Lines 156 to 169 in 120f198

def DeletePage(self, page):

"""Delete page

:param page: name

:return: True if page was deleted, False if not exists

"""

delPageIndex = self.GetPageIndexByName(page)

if delPageIndex != -1:

ret = self.classObject.DeletePage(self.widget, delPageIndex)

if ret:

del self.notebookPages[page]

return ret

else:

return False

FlatNotebookController class, GetPageIndexByName method

grass/gui/wxpython/gui_core/widgets.py

Lines 258 to 264 in 120f198

def GetPageIndexByName(self, page):

"""Get notebook page index

:param page: name

"""

if page not in self.notebookPages:

return -1

GNotebook class, method DeleteNBPage implementation allow delete page which was inserted with name param only.

tmszi · 2023-09-27T06:39:57Z

Rebase 327d334.

…usage

tmszi · 2023-09-30T08:12:34Z

@petrasovaa I refactored the use of InsertNBPage() (method use name param arg) method inside wxGUI/dbmgr component. Now DeleteNBPage() works as expected. Tested succesfully as usual.

This reverts commit fec2406.

tmszi · 2023-09-30T12:25:49Z

Wouldn't it be possibly better to adjust the code of this function to work with the GNotebook's version of DeletNBPage?

With 086b5b2 I refactor GetPageIndexByName() method of FlatNotebookController and of NotebookController class to meet your suggention. It is now possible to delete a notebook page by name or by page index position depending on how the page was added or inserted.

petrasovaa

Thank you, I think this is better!

…ta and Manage tables page (tab) (OSGeo#2422)

tmszi added bug Something isn't working backport_needed GUI wxGUI related labels Jun 9, 2022

tmszi added this to the 8.2.1 milestone Jun 9, 2022

petrasovaa reviewed Jun 10, 2022

View reviewed changes

neteler modified the milestones: 8.2.1, 8.2.2 Jan 22, 2023

wenzeslaus modified the milestones: 8.2.2, 8.3.1 Jun 6, 2023

landam added backport to 8.3 and removed backport_needed labels Jun 6, 2023

wxGUI/dbmgr: delete Notebook page with DeletePage() method

327d334

tmszi force-pushed the wxgui_dbmgr_fix_remove_layer branch from a4419dd to 327d334 Compare September 27, 2023 06:39

tmszi requested a review from petrasovaa September 27, 2023 06:40

tmszi added 2 commits September 30, 2023 09:50

Fix delete NB Browse, Tables page if layer was deleted

fec2406

wxGUI/widgets: update NotebookController class DeletePage method doc …

c4ee9e7

…usage

tmszi added 2 commits September 30, 2023 13:15

Revert "Fix delete NB Browse, Tables page if layer was deleted"

bd0b03c

This reverts commit fec2406.

Fix delete NB Browse, Tables page by page index if layer was deleted

086b5b2

petrasovaa approved these changes Oct 3, 2023

View reviewed changes

tmszi merged commit df6e6ae into OSGeo:main Oct 3, 2023

tmszi added a commit to tmszi/grass that referenced this pull request Oct 3, 2023

wxGUI/dbmgr: fix removing map table layer and layer related Browse da…

87cff41

…ta and Manage tables page (tab) (OSGeo#2422)

tmszi added a commit to tmszi/grass that referenced this pull request Oct 3, 2023

wxGUI/dbmgr: fix removing map table layer and layer related Browse da…

d9ea6d3

…ta and Manage tables page (tab) (OSGeo#2422)

tmszi deleted the wxgui_dbmgr_fix_remove_layer branch October 3, 2023 19:08

tmszi removed the backport to 8.3 label Oct 3, 2023

landam pushed a commit to landam/grass that referenced this pull request Oct 25, 2023

wxGUI/dbmgr: fix removing map table layer and layer related Browse da…

777f9f7

…ta and Manage tables page (tab) (OSGeo#2422)

neteler pushed a commit to nilason/grass that referenced this pull request Nov 7, 2023

wxGUI/dbmgr: fix removing map table layer and layer related Browse da…

ff01009

…ta and Manage tables page (tab) (OSGeo#2422)

HuidaeCho pushed a commit to HuidaeCho/grass that referenced this pull request Jan 9, 2024

wxGUI/dbmgr: fix removing map table layer and layer related Browse da…

31a09b6

…ta and Manage tables page (tab) (OSGeo#2422)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Comments

wxGUI/dbmgr: fix removing map table layer and layer related Browse data and Manage tables page (tab)#2422

wxGUI/dbmgr: fix removing map table layer and layer related Browse data and Manage tables page (tab)#2422
tmszi merged 5 commits intoOSGeo:mainfrom
tmszi:wxgui_dbmgr_fix_remove_layer

tmszi commented Jun 9, 2022

Uh oh!

petrasovaa Jun 10, 2022

Uh oh!

tmszi Jun 10, 2022 •

edited

Loading

Uh oh!

tmszi commented Sep 27, 2023

Uh oh!

tmszi commented Sep 30, 2023

Uh oh!

tmszi commented Sep 30, 2023 •

edited

Loading

Uh oh!

petrasovaa left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

	def InsertPage(self, args, *kwargs):
	"""Insert a new page"""
	if "name" in kwargs:
	self.notebookPages[kwargs["name"]] = kwargs["page"]
	del kwargs["name"]

	try:
	self.classObject.InsertPage(self.widget, args, *kwargs)
	except TypeError as e: # documentation says 'index', but certain versions of wx require 'n'
	kwargs["n"] = kwargs["index"]
	del kwargs["index"]
	self.classObject.InsertPage(self.widget, args, *kwargs)

	self.InsertNBPage(
	index=pos,
	page=panel,
	text=" %d / %s %s"
	% (layer, label, self.dbMgrData["mapDBInfo"].layers[layer]["table"]),
	)

	def DeletePage(self, page):
	"""Delete page

	:param page: name
	:return: True if page was deleted, False if not exists
	"""
	delPageIndex = self.GetPageIndexByName(page)
	if delPageIndex != -1:
	ret = self.classObject.DeletePage(self.widget, delPageIndex)
	if ret:
	del self.notebookPages[page]
	return ret
	else:
	return False

	def GetPageIndexByName(self, page):
	"""Get notebook page index

	:param page: name
	"""
	if page not in self.notebookPages:
	return -1

Uh oh!

Comments

Conversation

tmszi commented Jun 9, 2022

Uh oh!

petrasovaa Jun 10, 2022

Choose a reason for hiding this comment

Uh oh!

tmszi Jun 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tmszi commented Sep 27, 2023

Uh oh!

tmszi commented Sep 30, 2023

Uh oh!

tmszi commented Sep 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

petrasovaa left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

tmszi Jun 10, 2022 •

edited

Loading

tmszi commented Sep 30, 2023 •

edited

Loading