Store list of databases in settings #858

lindakarlovska · 2020-07-30T16:19:48Z

The GUI should remember the databases added. Every time a new database is added, a list of databases in the user settings should be updated. This way, a new GUI instance should read the user settings and put the databases to the list (if the directories still exist). The GUI should allow to remove the database from the tree. This should again be reflected in the user settings.

…nteractively which removes selected grass db. The current grassdb cannot be removed. It is considered as well.

lindakarlovska · 2020-07-30T16:23:56Z

Now it is possible to click on grassdb node and selected "Remove database from the tree". The current GRASS database cannot be removed.

The next step will be to put the databases to the user settings list or remove it (according to performed operation).

mlennert · 2020-07-31T04:45:32Z

Maybe the message could be a bit more explicit:

"Do you want to remove the following GRASS GIS database from the data catalogue ?"

"The directory itself will not be deleted from the disk."

Or something similar ?

lindakarlovska · 2020-07-31T06:11:52Z

Moritz, We have eventually agreed with Anicka that the message will not be needed at all in this case since the user can add removed database to the tree whenever again. The message is important when one deletes data. So I will create within this PR the function for real database deleting as well and this function will include the message box. What do you think, does that make sense? 31. 7. 2020 v 6:45, mlennert <notifications@github.com>:

…

Maybe the message could be a bit more explicit: "Do you want to remove the following GRASS GIS database from the data catalogue ?" "The directory itself will not be deleted from the disk." Or something similar ? — You are receiving this because you were assigned. Reply to this email directly, view it on GitHub, or unsubscribe.

mlennert · 2020-07-31T06:41:51Z

Yes, perfectly !

…ssdb path are now stored into UserSettings. Function InsertGrassDb edited to add only grassdb which does not currently exist in the data catalogue.

lindakarlovska · 2020-07-31T10:32:22Z

If I start GRASS for the first time (withnout wx file stored in .grass7) everything works as expected. The Wx file is automatically created and includes paths to all currently existing grassdatabases in the data catalogue.

There is a problem while starting GRASS for the second time (when wx file already exists). I get the following error:

Not sure how to go ahead.

…GrassDb function.

mlennert · 2020-08-03T13:49:52Z

If I start GRASS for the first time (withnout wx file stored in .grass7) everything works as expected. The Wx file is automatically created and includes paths to all currently existing grassdatabases in the data catalogue.

There is a problem while starting GRASS for the second time (when wx file already exists). I get the following error:

Try to not use screenshots to show error messages. Just copy them as text. It makes it easier to react to (and quote) them.

Not sure how to go ahead.

With a freshly checked out master, and just applying your PR here, and the entire .grass7 erased, I do not get the above error message and I can successfully add and remove databases and the list of current databases is correctly maintained in the .grass7/wx file.

At one point I saw the error message you got, but I haven't been able to reproduce when I start with a fresh .grass7 directory (probably just a fresh wx file is probably enough).

When you set the GUI language to 'system', the line in the wx file should apparently read:

language;locale;lc_all;None

If the line still has 'system' written into it, this causes the error message.

But your PR seems to work fine.

lindakarlovska · 2020-08-03T14:17:28Z

If I start GRASS for the first time (withnout wx file stored in .grass7) everything works as expected. The Wx file is automatically created and includes paths to all currently existing grassdatabases in the data catalogue.
There is a problem while starting GRASS for the second time (when wx file already exists). I get the following error:

Try to not use screenshots to show error messages. Just copy them as text. It makes it easier to react to (and quote) them.

Not sure how to go ahead.

With a freshly checked out master, and just applying your PR here, and the entire .grass7 erased, I do not get the above error message and I can successfully add and remove databases and the list of current databases is correctly maintained in the .grass7/wx file.

At one point I saw the error message you got, but I haven't been able to reproduce when I start with a fresh .grass7 directory (probably just a fresh wx file is probably enough).

When you set the GUI language to 'system', the line in the wx file should apparently read:

language;locale;lc_all;None

If the line still has 'system' written into it, this causes the error message.

But your PR seems to work fine.

Yes, when erasing the entire .grass7, I do not get the above error, either.
Good. I will let this error be for now. :-)

…adable.

lindakarlovska · 2020-08-04T08:20:17Z

I made code more readable and clear but the error when starting grass for the second time is the same:

_Failed to enforce user specified language 'system' with error: 'unsupported locale setting'
A LANGUAGE environmental variable has been set.
Part of messages will be displayed in the requested language.
Starting GRASS GIS...
WARNING: Ignoring unsupported debug level (must be >=0 and <=5). unknown locale: system
WARNING: Ignoring unsupported wx debug level (must be >=0 and <=5). unknown locale: system
Traceback (most recent call last):
File "/home/linwe/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/gis_set.py", line 37, in
from core.gcmd import GError, RunCommand
File "/home/linwe/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/core/gcmd.py", line 781, in
_enc = GetDefaultEncoding() # define as global variable
File "/home/linwe/grass/dist.x86_64-pc-linux-gnu/gui/wxpython/core/gcmd.py", line 771, in GetDefaultEncoding
enc = locale.getdefaultlocale()[1]
File "/usr/lib/python3.6/locale.py", line 562, in getdefaultlocale
return _parse_localename(localename)
File "/usr/lib/python3.6/locale.py", line 490, in parse_localename
raise ValueError('unknown locale: %s' % localename)
ValueError: unknown locale: system
ERROR: Error in GUI startup. See messages above (if any) and if necessary, please report this error to the GRASS developers.
On systems with package manager, make sure you have the right GUI package, probably named grass-gui, installed.
To run GRASS GIS in text mode use the --text flag.
Use '--help' for further options
grass79 --help
See also: https://grass.osgeo.org/grass79/manuals/helptext.html
Exiting...

I will let this error be for now.

mlennert · 2020-08-04T09:30:13Z

Have you tried recompiling all of GRASS GIS ? With a 'make distclean' before the configure+make ?

mlennert · 2020-08-04T10:22:27Z

Have you tried recompiling all of GRASS GIS ? With a 'make distclean' before the configure+make ?

Or, to start with a ligher solution: find the line

language;locale;lc_all;None

in your wx config file.

From your error message, I imagine that the last element is 'system' and not 'None' ? Try changing it to 'None'.

mlennert · 2020-08-04T10:45:46Z

Or you could try this in the Python console and see the result:

import locale
locale.getdefaultlocale()[1]

lindakarlovska · 2020-08-04T11:19:25Z

Have you tried recompiling all of GRASS GIS ? With a 'make distclean' before the configure+make ?

Or, to start with a ligher solution: find the line

language;locale;lc_all;None

in your wx config file.

From your error message, I imagine that the last element is 'system' and not 'None' ? Try changing it to 'None'.

Great! I have tried that and it works.

Observed in the wild by Linda Kladivova OSGeo#858 (comment)

marisn · 2020-08-05T06:55:09Z

I made code more readable and clear but the error when starting grass for the second time is the same:

_Failed to enforce user specified language 'system' with error: 'unsupported locale setting'

That's strange. 'system' should not creep into wx file. I created a pull request to handle such strange wx files: #877

grass/gui/wxpython/gui_core/preferences.py

Line 186 in bc417cd

if lang == 'system':

petrasovaa · 2020-07-31T17:02:08Z

gui/wxpython/core/settings.py

@@ -112,6 +112,10 @@ def _defaultSettings(self):
                        'enabled' : False
                    },
                },
+                # datacatalog


Shouldn't this be on the same level as eg. 'manager'?

Moved. UserSettings needs to have three values so that now it is:

group="datacatalog",
key="grassdb"
subkey="getString"

petrasovaa · 2020-07-31T17:09:07Z

gui/wxpython/core/settings.py

@@ -112,6 +112,10 @@ def _defaultSettings(self):
                        'enabled' : False
                    },
                },
+                # datacatalog
+                'datacatalog': {
+                    'grassdb': None


Maybe empty string than None for consistent types

petrasovaa · 2020-07-31T17:11:21Z

gui/wxpython/datacatalog/catalog.py

        dlg = wx.DirDialog(self, _("Choose GRASS data directory:"),
                           os.getcwd(), wx.DD_DEFAULT_STYLE)
        if dlg.ShowModal() == wx.ID_OK:
            grassdatabase = dlg.GetPath()
-            self.tree.InsertGrassDb(name=grassdatabase)
-
+            grassdb_node = self.tree._model.SearchNodes(name=grassdatabase,


Perhaps this could go into InsertGrassDb directly to avoid code duplication?

petrasovaa · 2020-07-31T17:12:30Z

gui/wxpython/datacatalog/catalog.py

+            grassdb_node = self.tree._model.SearchNodes(name=grassdatabase,
+                                                   type='grassdb')
+            if grassdb_node:
+                dlg = wx.MessageDialog(


I am thinking maybe we don't have to inform user about this, just check and see if to add it or no.

petrasovaa · 2020-07-31T17:13:04Z

gui/wxpython/datacatalog/tree.py

@@ -254,8 +256,27 @@ def __init__(
        self._initVariablesCatalog()
        self.UpdateCurrentDbLocationMapsetNode()

-        self.grassdatabases = []
-        self.grassdatabases.append(gisenv()['GISDBASE'])
+        self.string_of_grassdatabases = UserSettings.Get(


This doesn't need to be self

Eventually It is enough to use just one variable "self.grassdatabases".

petrasovaa · 2020-08-05T18:57:32Z

I think I know what's the problem with 'system'. Saving settings properly is not well documented, the procedure is more complicated (and the entire settings should be rewritten for better API and format). Basically you need to first load the settings from file, change the parts you want and then save it. This way, you won't get the 'system' there, because that's prevented in core/preferences.py. So taking an example from vdigit:

        UserSettings.Set(...)
        fileSettings = {}
        UserSettings.ReadSettingsFile(settings=fileSettings)
        fileSettings['vdigit'] = UserSettings.Get(group='vdigit')
        UserSettings.SaveToFile(fileSettings)

This is the way to update the wx file without writing there any other current /temporary settings. So @lindakladivova please adapt this and make it into a function in tree.py.

I would say #877 is not needed, it would hide the real problem.

petrasovaa · 2020-08-05T18:59:11Z

Also I posted some comments just now, although I made them couple days ago and apparently forgot to hit the submit button, so hopefully they still make sense.

lindakarlovska · 2020-08-06T13:02:28Z

I think I know what's the problem with 'system'. Saving settings properly is not well documented, the procedure is more complicated (and the entire settings should be rewritten for better API and format). Basically you need to first load the settings from file, change the parts you want and then save it. This way, you won't get the 'system' there, because that's prevented in core/preferences.py. So taking an example from vdigit:
        UserSettings.Set(...)
        fileSettings = {}
        UserSettings.ReadSettingsFile(settings=fileSettings)
        fileSettings['vdigit'] = UserSettings.Get(group='vdigit')
        UserSettings.SaveToFile(fileSettings)
This is the way to update the wx file without writing there any other current /temporary settings. So @lindakladivova please adapt this and make it into a function in tree.py.

I would say #877 is not needed, it would hide the real problem.

Anna, I am quite a desperate :/ do not know how to do this. There is a problem that first of all I need to load current grassdb string from wx file:
# Load grassdatabases from user's settings
self.grassdatabases = (UserSettings.Get(
group='datacatalog',
key='grassdb',
subkey='getString'))
I cannot use UserSettings.Set(..), I need to use User.Settings.get(...)..

lindakarlovska · 2020-08-06T13:04:38Z

I committed the new version with some improvements which Anna mentioned but I did not manage solving the "system" problem.

…ger', kept empty string for consistent types, InsertGrassDb updated, user is not informed about adding an existing database.

petrasovaa · 2020-08-06T20:17:28Z

@lindakladivova could you test it now and I think if it works we can merge this.

lindakarlovska · 2020-08-07T12:19:24Z

@lindakladivova could you test it now and I think if it works we can merge this.

Works perfectly. Let's merge it.

marisn · 2020-08-12T14:17:00Z

I would say #877 is not needed, it would hide the real problem.

IMHO it doesn't do any harm to have that workaround there.

petrasovaa · 2020-08-12T15:28:09Z

I would say #877 is not needed, it would hide the real problem.

IMHO it doesn't do any harm to have that workaround there.

Hiding the problem is in my opinion doing harm... Next time a programmer uses the settings in a wrong way, they won't spot they did something wrong.

marisn · 2020-08-13T13:27:18Z

Hiding the problem is in my opinion doing harm... Next time a programmer uses the settings in a wrong way, they won't spot they did something wrong.

There should be a better way how to spot improper use of settings API than relaying on settings file corruption in a particular way.

petrasovaa · 2020-08-13T14:57:34Z

Hiding the problem is in my opinion doing harm... Next time a programmer uses the settings in a wrong way, they won't spot they did something wrong.

There should be a better way how to spot improper use of settings API than relaying on settings file corruption in a particular way.

That's not what I am saying. The change you suggest is not needed if the API is used as intended. (The API is weird and should be changed but that's a different discussion). We don't normally check for every misuse of APIs because that's programmer's error. If you want to have the change there, it must be clearly documented why was it added there, otherwise next time somebody looks at the code, they will be confused and the code will bloat.
I didn't want to argue about this rather insignificant issue, but I feel it's a general problem.

mlennert · 2020-08-14T08:56:03Z

I agree with Anna on this one. My first reaction was based on my lack of knowledge of the functioning of the settings system.

wxGUI/datacatalog: Added functions RemoveGrassDb and remove_grassdb_i…

ea354dc

…nteractively which removes selected grass db. The current grassdb cannot be removed. It is considered as well.

lindakarlovska marked this pull request as draft July 30, 2020 16:24

wenzeslaus linked an issue Jul 30, 2020 that may be closed by this pull request

[Feat] Store list of databases in settings #842

Closed

wenzeslaus added this to PRs in progress in New GUI Startup via automation Jul 30, 2020

petrasovaa assigned lindakarlovska Jul 30, 2020

wxGUI/datalog: Functions for removing and deleting grassdb added. Gra…

7531d28

…ssdb path are now stored into UserSettings. Function InsertGrassDb edited to add only grassdb which does not currently exist in the data catalogue.

wxGUI/datacatalog: The check for already added grassdb moved to OnAdd…

0d067c2

…GrassDb function.

wxGUI/datacatalog: Storing list of databases in settings made more re…

9d5c4e5

…adable.

marisn added a commit to marisn/grass that referenced this pull request Aug 5, 2020

init: Do not try to set system as a GRASS session locale

14a83e0

Observed in the wild by Linda Kladivova OSGeo#858 (comment)

marisn mentioned this pull request Aug 5, 2020

init: Do not try to set system as a GRASS session locale #877

Closed

lindakarlovska added enhancement New feature or request gsoc Reserved for Google Summer of Code student(s) GUI wxGUI related labels Aug 5, 2020

lindakarlovska marked this pull request as ready for review August 5, 2020 11:54

petrasovaa reviewed Aug 5, 2020

View reviewed changes

lindakladivova and others added 5 commits August 6, 2020 08:17

wxGUI/startup: datacatalog settings moved to same level as e.g. 'mana…

8405f68

…ger', kept empty string for consistent types, InsertGrassDb updated, user is not informed about adding an existing database.

wxGUI/startup: Flake corrected.

59cb358

wxGUI/startup: tree/frame.py updated.

c4ce884

load and save grass dbs from settings

defad89

Merge branch 'master' into wxGUI-Store-list-of-databases-in-settings

7d683bc

petrasovaa merged commit 0d8ea92 into OSGeo:master Aug 8, 2020

New GUI Startup automation moved this from PRs in progress to Done Aug 8, 2020

neteler added this to the 8.0.0 milestone Dec 9, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Store list of databases in settings #858

Store list of databases in settings #858

lindakarlovska commented Jul 30, 2020

lindakarlovska commented Jul 30, 2020

mlennert commented Jul 31, 2020

lindakarlovska commented Jul 31, 2020 via email

mlennert commented Jul 31, 2020

lindakarlovska commented Jul 31, 2020 •

edited

Loading

mlennert commented Aug 3, 2020

lindakarlovska commented Aug 3, 2020

lindakarlovska commented Aug 4, 2020 •

edited

Loading

mlennert commented Aug 4, 2020

mlennert commented Aug 4, 2020

mlennert commented Aug 4, 2020

lindakarlovska commented Aug 4, 2020

marisn commented Aug 5, 2020

petrasovaa Jul 31, 2020

lindakarlovska Aug 6, 2020 •

edited

Loading

petrasovaa Jul 31, 2020

lindakarlovska Aug 6, 2020

petrasovaa Jul 31, 2020

lindakarlovska Aug 6, 2020

petrasovaa Jul 31, 2020

lindakarlovska Aug 6, 2020

petrasovaa Jul 31, 2020

lindakarlovska Aug 6, 2020 •

edited

Loading

petrasovaa commented Aug 5, 2020

petrasovaa commented Aug 5, 2020

lindakarlovska commented Aug 6, 2020 •

edited

Loading

lindakarlovska commented Aug 6, 2020

petrasovaa commented Aug 6, 2020

lindakarlovska commented Aug 7, 2020

marisn commented Aug 12, 2020

petrasovaa commented Aug 12, 2020

marisn commented Aug 13, 2020

petrasovaa commented Aug 13, 2020

mlennert commented Aug 14, 2020

Store list of databases in settings #858

Store list of databases in settings #858

Conversation

lindakarlovska commented Jul 30, 2020

lindakarlovska commented Jul 30, 2020

mlennert commented Jul 31, 2020

lindakarlovska commented Jul 31, 2020 via email

mlennert commented Jul 31, 2020

lindakarlovska commented Jul 31, 2020 • edited Loading

mlennert commented Aug 3, 2020

lindakarlovska commented Aug 3, 2020

lindakarlovska commented Aug 4, 2020 • edited Loading

mlennert commented Aug 4, 2020

mlennert commented Aug 4, 2020

mlennert commented Aug 4, 2020

lindakarlovska commented Aug 4, 2020

marisn commented Aug 5, 2020

Choose a reason for hiding this comment

lindakarlovska Aug 6, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lindakarlovska Aug 6, 2020 • edited Loading

Choose a reason for hiding this comment

petrasovaa commented Aug 5, 2020

petrasovaa commented Aug 5, 2020

lindakarlovska commented Aug 6, 2020 • edited Loading

lindakarlovska commented Aug 6, 2020

petrasovaa commented Aug 6, 2020

lindakarlovska commented Aug 7, 2020

marisn commented Aug 12, 2020

petrasovaa commented Aug 12, 2020

marisn commented Aug 13, 2020

petrasovaa commented Aug 13, 2020

mlennert commented Aug 14, 2020

lindakarlovska commented Jul 31, 2020 •

edited

Loading

lindakarlovska commented Aug 4, 2020 •

edited

Loading

lindakarlovska Aug 6, 2020 •

edited

Loading

lindakarlovska Aug 6, 2020 •

edited

Loading

lindakarlovska commented Aug 6, 2020 •

edited

Loading