wxgui: Create grassdata automatically on the first GUI startup

[lindakarlovska]

After starting GRASS GIS with GUI, the GUI now searches for existing grassdata (e.g., dac6d4a and #644 with #664). If nothing is found, GRASS GIS (GUI) should automatically create directory named grassdata as a subdirectory of a platfrom-dependent directory. This platfrom-dependent directory would be:

• $HOME (os.path.expanduser('~')) on Linux and the like,
• User's Documents on Windows (see dac6d4a for code trying to identify that dir),
• One of the above on macOS - macOS users, please share your ideas.

If that fails (GRASS is running in some special environment and the standard directories cannot be created which might be the case in some "try out GRASS" use cases) it should use a temporary directory (/tmp/... etc.) as a fallback.
The created tmp is not cleaned by GRASS, so we are relying on the system to do it at some point.

[landam]

Why _() in the path? Localized Windows should understand Documents.

[wenzeslaus]

This comes from dac6d4a - see the comment there. I would not bother with that for this function and just go with Documents only.

[wenzeslaus]

As I said, here in this new function, I would not bother with translations here as this is more future oriented and we hope that every new system will be set in a way that there is "Documents". So, I'm saying, delete the "candidates.append(os.path.join(home, _("Documents"), "grassdata"))" part.

[wenzeslaus]

The general structure is good, but there is a lot of smaller issues which need to be addressed. (See the individual comments.)

You can just go ahead and make macOS behave same as Linux as that's what @nilason suggests in #682.

[wenzeslaus]

This goes against "If even [the temporary directory fallback] fails, GRASS should not fail, but simply let the user make that choice which is the current behavior." from #682.

[wenzeslaus]

This should be created only once it is really needed. (Otherwise, the tmp dir will be created and not deleted every time thing function runs.)

[wenzeslaus]

The path is already set at this point even when exception was raised, so your line if path is None: will have path which is not None and thus the functions will just return this path.

[wenzeslaus]

We may decide to refine it and give more info to the user through GUI, but at this point and in this function, there is no need to report the failure in any way. This function has a fallback which is the tmp dir and the original calling code has its own fallback solution.

[wenzeslaus]

This comes from dac6d4a - see the comment there. I would not bother with that for this function and just go with Documents only.

[wenzeslaus]

This still should be there. (#682: "If even [the temporary directory fallback] fails, [...] simply let the user make that choice which is the current behavior.")

[wenzeslaus]

possible -> standard, fix wording of "directory to what is"

[wenzeslaus]

Something like "...in the standard location according to the platform." And also mention the fallback.

[wenzeslaus]

Please remove this extra change. Also in PEP8, there is only one empty line between methods.

[wenzeslaus]

Based on what the function does and what the context is, the comment should be "...try to create..."

[wenzeslaus]

What the word "independent" refers to here?

[wenzeslaus]

As I said, here in this new function, I would not bother with translations here as this is more future oriented and we hope that every new system will be set in a way that there is "Documents". So, I'm saying, delete the "candidates.append(os.path.join(home, _("Documents"), "grassdata"))" part.

[wenzeslaus]

Here, you don't have to assign to a variable here, just return candidate right away.

[wenzeslaus]

According to PEP8, t from tempfile and " from "grassdata... should be aligned. Another option is to use Black style which puts this into 4 lines if it is longer than 88 characters:

tmp = os.path.join(
    tempfile.gettempdir(),
    "grassdata_{}".format(getpass.getuser())
) 

You can run Flake8 on any file you edit to see these things. Unfortunately, you will get a lot of warning and errors from other lines you did not edit and you need to ignore those (this will get better in the future as we make more files PEP8, Flake8, and Black compliant).

[wenzeslaus]

Again, just return here is enough.

[wenzeslaus]

Only return is needed here, no assignment.

[wenzeslaus]

Control -> Check

[wenzeslaus]

create_possible_database_path() -> create_database_directory()

The GRASS terminology here is all over the place here (grassdata, gisbase, ...), but "possible" does not make sense here and we are not creating a path, but the actual directory, so that's way "...database_directory".

[wenzeslaus]

Since the true branch of the if statement has return, you don't need else here and can move the try-except block one level left. This is does not change anything, but is makes the code simpler and the same logic is already used above: The good case returns, so the other case is just whatever follows after that.

[wenzeslaus]

Simple candidate = os.path... will suffice here in both cases. It is either or and at one point you have only one candidate. This removes the need for the loop over the candidates list below.

[wenzeslaus]

Only minor comments to make the code more readable.

[wenzeslaus]

The "case independent" does not make sense here. Simplify further, by merging the two comments together and saying something like "Determine the standard path according to a platform."

While you are at it, you can drop the "candidate" terminology from the variable name too. What candidate refers to here? It really makes sense only in the other function. Simple path will be probably more descriptive here. And going further, the tmp variable can be called path too because in a way it is no different from the path before, it replaces it for all intents and purposes.

[wenzeslaus]

The intended behavior may not be clear from the code, so add another comment saying something like "The created tmp is not cleaned by GRASS, so we are relying on the system to do it at some point. The positive outcome is that another GRASS instance will find the data created by the first one which is desired in the "try out GRASS" use case we are aiming towards." See the original ticket for alternative formulation of this.

[wenzeslaus]

Say something like "Create a temporary "grassdata" directory if GRASS is running in some special environment and the standard directories cannot be created which might be the case in some "try out GRASS" use cases."

(Note that a line should be 88 characters or less.)

[wenzeslaus]

Catching trailing whitespace.

Use flake8 myfile.py to check the contents of files you edit. Ignore what is reported for lines you did not touch. Fix what is reported for lines you are changing. Use the https://www.flake8rules.com/ website for explanations. Ask if something is not clear. Not all the rules can be applied to the whole GRASS code base, but most of them should.

[wenzeslaus]

Trailing whitespace (Flake8 W291)

[wenzeslaus]

Trailing whitespace (Flake8 W291)

[wenzeslaus]

Trailing whitespace (Flake8 W291)

[wenzeslaus]

Trailing whitespace (Flake8 W291)

[wenzeslaus]

Trailing whitespace (Flake8 W291)

[wenzeslaus]

Trailing whitespace (Flake8 W291)

[wenzeslaus]

Trailing whitespace (Flake8 W291)

[petrasovaa]

Hopefully the last change, sorry I missed this before: there are other platforms (see https://stackoverflow.com/questions/446209/possible-values-from-sys-platform) so we should probably have it more like this (and treat windows as special case):

if sys.platform == 'win32':
    path = os.path.join(home, "Documents", "grassdata")
else:
    path = os.path.join(home, "grassdata")