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.

Sign up for GitHub

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

Jump to bottom

Add rename and delete of Location and Mapset in Datacatalog #771

Merged
merged 13 commits into from
Jul 14, 2020
Merged

Add rename and delete of Location and Mapset in Datacatalog #771

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
4ce0821
wxGUI/datacatalog: Deleting of mapset and location, for current ones …
Jul 6, 2020
b73f8e9
wxGUI/datacatalog: Renaming of location corrected. Importing of packa…
Jul 7, 2020
b6f7dc2
wxGUI/datacatalog: Created general functions for create, rename and d…
Jul 9, 2020
b7225a7
wxGUI/datacatalog: Flake8 corrected.
Jul 9, 2020
d81467a
wxGUI/datacatalog: SwitchLocationMapset in tree.py edited but still n…
Jul 10, 2020
c1f930e
wxGUI/datacatalog: _nameValidationFailed function removed from gis_se…
Jul 10, 2020
9415c5e
wxGUI/datacatalog: Added check to prevent user from renaming a curren…
Jul 11, 2020
72989fa
wxGUI/datacatalog: Returns in guiutils.py corrected. The error after …
Jul 12, 2020
ab3cbd9
Merge branch 'master' into wxGUI-datacatalog-Add-rename-and-delete-of…
petrasovaa Jul 13, 2020
9e9937e
Merge remote-tracking branch 'upstream/master' into wxGUI-datacatalog…
wenzeslaus Jul 14, 2020
4d942f7
Update UpdateCurrentLocationMapsetNode name
wenzeslaus Jul 14, 2020
5b43ab3
Use db from the tree, not the current one
wenzeslaus Jul 14, 2020
611d133
Use db in checks for current loc or mapset
wenzeslaus Jul 14, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 123 additions & 26 deletions gui/wxpython/datacatalog/tree.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,11 @@
from gui_core.wrap import Menu
from datacatalog.dialogs import CatalogReprojectionDialog
from icons.icon import MetaIcon
from startup.utils import create_mapset, get_default_mapset_name
from startup.guiutils import NewMapsetDialog
from startup.guiutils import (create_mapset_interactively,
rename_mapset_interactively,
rename_location_interactively,
delete_mapset_interactively,
delete_location_interactively)

from grass.pydispatch.signal import Signal

Expand Down Expand Up @@ -636,6 +639,63 @@ def OnRenameMap(self, event):
if new_name:
self.Rename(old_name, new_name)

def OnCreateMapset(self, event):
"""Create new mapset"""
gisdbase = gisenv()['GISDBASE']
try:
mapset = create_mapset_interactively(self,
gisdbase,
self.selected_location[0].label)
self.InsertMapset(name=mapset,
location_node=self.selected_location[0])
return True
except Exception as e:
GError(parent=self,
message=_("Unable to create new mapset: %s") % e,
showTraceback=False)
return False

def OnRenameMapset(self, event):
wenzeslaus marked this conversation as resolved.
Show resolved Hide resolved
"""
Rename selected mapset
"""
try:
newmapset = rename_mapset_interactively(
self,
gisenv()['GISDBASE'],
self.selected_location[0].label,
self.selected_mapset[0].label)
# When renaming a current mapset, switch to a new mapset
if (self.selected_mapset[0].label == gisenv()['MAPSET']):
self.selected_mapset[0].label = newmapset
self._SwitchLocationMapset()
except Exception as e:
GError(parent=self,
message=_("Unable to rename mapset: %s") % e,
showTraceback=False)
return False
self.ReloadTreeItems()

def OnRenameLocation(self, event):
"""
Rename selected location
"""
try:
newlocation = rename_location_interactively(
self,
gisenv()['GISDBASE'],
self.selected_location[0].label)
# When renaming a current location, switch to a new location
if (self.selected_location[0].label == gisenv()['LOCATION_NAME']):
self.selected_location[0].label = newlocation
self._SwitchLocationMapset(new_location=newlocation)
except Exception as e:
GError(parent=self,
message=_("Unable to rename location: %s") % e,
showTraceback=False)
return False
self.ReloadTreeItems()

def OnStartEditLabel(self, node, event):
"""Start label editing"""
self.DefineItems([node])
Expand Down Expand Up @@ -839,6 +899,37 @@ def OnDeleteMap(self, event):
self.UnselectAll()
self.showNotification.emit(message=_("g.remove completed"))

def OnDeleteMapset(self, event):
"""
Delete selected mapset
"""
try:
delete_mapset_interactively(self,
gisenv()['GISDBASE'],
self.selected_location[0].label,
self.selected_mapset[0].label)
except Exception as e:
GError(parent=self,
message=_("Unable to delete mapset: %s") % e,
showTraceback=False)
return False
self.ReloadTreeItems()

def OnDeleteLocation(self, event):
"""
Delete selected location
"""
try:
delete_location_interactively(self,
gisenv()['GISDBASE'],
self.selected_location[0].label)
except Exception as e:
GError(parent=self,
message=_("Unable to delete location: %s") % e,
showTraceback=False)
return False
self.ReloadTreeItems()

def OnDisplayLayer(self, event):
"""Display layer in current graphics view"""
self.DisplayLayer()
Expand Down Expand Up @@ -889,38 +980,21 @@ def OnEndDrag(self, node, event):
self.OnPasteMap(event)

def OnSwitchLocationMapset(self, event):
"""Switch to location and mapset"""
self._SwitchLocationMapset()

def _SwitchLocationMapset(self):
"""Switch to location and mapset"""
genv = gisenv()
if self.selected_location[0].label == genv['LOCATION_NAME']:
self.changeMapset.emit(mapset=self.selected_mapset[0].label)
else:
self.changeLocation.emit(mapset=self.selected_mapset[0].label, location=self.selected_location[0].label)
self.changeLocation.emit(mapset=self.selected_mapset[0].label,
location=self.selected_location[0].label)
self.UpdateCurrentLocationMapsetNode()
self.ExpandCurrentMapset()
self.RefreshItems()
Copy link
Member

Choose a reason for hiding this comment

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

This looks like a good start. Now you can maybe make use of the option of calling _SwitchLocationMapset() with different parameters based on the the context. However, still:

I would need you to document your intentions here in the code to understand what this is supposed to solve. Documentation and comments describing the intent are often useful even when writing the code as they force us to think about the code is supposed to do. For example, for a line add_to_list(abc) we should aim at answering questions such as: Why we are adding to the list? What is the intended outcome? What are the possible outcomes? What are the assumptions about abc or other things before add_to_list() is called?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Vashek, ok, I have a function OnRenameMapset that calls function rename_mapset_interactively and returns new mapset name. I think this is implemented right.
But in the case of "if (self.selected_mapset[0].label == gisenv()['MAPSET'])" I need to switch my old mapset to the new one because my current mapset has been actually changed by renaming. But not literally so I need to self.selected_mapset[0].label = newmapset and then call the self._SwitchLocationMapset() function which can be generally the same for both functions OnRenameMapset, OnRenameLocation.
Where is the mistake in my thinking?

Copy link
Member

Choose a reason for hiding this comment

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

...and _SwitchLocationMapset() uses the currently selected location/mapset to switch to. That all makes sense and should work. So, we have to continue to explore the actual behavior:

When I test renaming the current mapset in GUI, it freezes, but first it shows an error message "Error in g.mapset" saying that the mapset "old_name" does not exist. So, it seems g.mapset is used for the switch and it requires the old mapset to exist.

When I create the same situation in the command line and use g.mapset, I get the same behavior. Also I confirmed that g.mapset is used in ChangeLocationMapset() in datacatalog/frame.py to switch the mapset.

The conclusion here is that mapset cannot be changed when the current does not exist, or in our case, we cannot switch into the one with the new name after we renamed it as long as we are using g.mapset. g.mapset checks correctness of the state of the database connection, i.e., the current mapset, but we are of course breaking that correctness by our rename, so g.mapset is not the right tool for the task.

Therefore, for this ticket, use the same check to prevent user from renaming the current location or mapset. This is something which could be added in the future, but it is a separate issue/additional feature.

The freeze with "Error in g.mapset" window is possibly a separate issue too, but something we should get back to (at least open an issue - please do that if you can reproduce it outside of your PR, e.g., by deleting a the mapset directory in a file manager right before you switch to it in the data catalog).


def OnCreateMapset(self, event):
"""Create new mapset"""
gisdbase = gisenv()['GISDBASE']

dlg = NewMapsetDialog(
parent=self,
default=get_default_mapset_name(),
database=gisdbase,
location=self.selected_location[0].label
)
if dlg.ShowModal() == wx.ID_OK:
mapset = dlg.GetValue()
try:
create_mapset(gisdbase,
self.selected_location[0].label,
mapset)
except OSError as err:
GError(parent=self,
message=_("Unable to create new mapset: %s") % err,
showTraceback=False)
self.InsertMapset(name=mapset,
location_node=self.selected_location[0])

def OnMetadata(self, event):
"""Show metadata of any raster/vector/3draster"""
def done(event):
Expand Down Expand Up @@ -1086,17 +1160,40 @@ def _popupMenuMapset(self):
if (self.selected_location[0].label == genv['LOCATION_NAME']
and self.selected_mapset[0].label == genv['MAPSET']):
item.Enable(False)

item = wx.MenuItem(menu, wx.ID_ANY, _("&Delete mapset"))
menu.AppendItem(item)
self.Bind(wx.EVT_MENU, self.OnDeleteMapset, item)
if (self.selected_location[0].label == genv['LOCATION_NAME']
and self.selected_mapset[0].label == genv['MAPSET']):
item.Enable(False)

item = wx.MenuItem(menu, wx.ID_ANY, _("&Rename mapset"))
menu.AppendItem(item)
self.Bind(wx.EVT_MENU, self.OnRenameMapset, item)

self.PopupMenu(menu)
menu.Destroy()

def _popupMenuLocation(self):
"""Create popup menu for locations"""
menu = Menu()
genv = gisenv()

item = wx.MenuItem(menu, wx.ID_ANY, _("&Create mapset"))
menu.AppendItem(item)
self.Bind(wx.EVT_MENU, self.OnCreateMapset, item)

item = wx.MenuItem(menu, wx.ID_ANY, _("&Delete location"))
menu.AppendItem(item)
self.Bind(wx.EVT_MENU, self.OnDeleteLocation, item)
if (self.selected_location[0].label == genv['LOCATION_NAME']):
item.Enable(False)

item = wx.MenuItem(menu, wx.ID_ANY, _("&Rename location"))
menu.AppendItem(item)
self.Bind(wx.EVT_MENU, self.OnRenameLocation, item)

self.PopupMenu(menu)
menu.Destroy()

Expand Down
Loading