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.

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

init: Create demolocation in grass.py, not wxGUI gis_set.py #1208

Merged
merged 3 commits into from Feb 18, 2021

Conversation

@wenzeslaus
Copy link
Member

@wenzeslaus wenzeslaus commented Dec 23, 2020

  • Moves functionality from gis_set.py to grass.py.
  • Moves library functions from wxGUI startup to new grass.app.
  • Opens the old startup when the PERMANENT of demolocation is not usable.
@wenzeslaus wenzeslaus added this to PRs in progress in New GUI Startup via automation Dec 23, 2020
@wenzeslaus wenzeslaus requested review from lindakladivova and landam Dec 23, 2020
@lindakladivova
Copy link
Contributor

@lindakladivova lindakladivova commented Dec 25, 2020

I think it is good. As I get it right, your already control if PERMANENT mapset in demolocation is in use. And if yes the startup screen is still used. So now we need to create a user mapset in this situation and start in demolocation, according to this proposal:
normal_user_diagram

After merging this I would create different PR dealing with permanent startup screen removal.

@wenzeslaus wenzeslaus requested a review from HuidaeCho Jan 13, 2021
@HuidaeCho
Copy link
Member

@HuidaeCho HuidaeCho commented Jan 14, 2021

@lindakladivova @wenzeslaus I think I'm missing something here. What is the rationale behind all this workflow for creating an unrequested location and mapset? If a last used mapset is in an unusable state (not clear what it means... because another user is already using it? Or has GRASS crashed before leaving the mapset in an "unusable" and unfixable state?), shouldn't we try to fix the mapset instead of creating a new default location and mapset silently? What if the default mapset in the default location becomes "unusable"? Create yet another default mapset 2 in that same default location or a default mapset in a new default location 2? And keep creating a new mapset whenever the last used default mapset becomes unusable?

The last step in yellow is also unclear. Does it create a new default location first and then what does it suggest to do in "To continue..."? If the user continues, what happens? If not, then what? Continue to switch to the last used mapset (is it possible now?)? Not continue to start in the just created default mapset?

Other than the workflow, moving code for non-GUI functionalities to grass.app looks fine. BTW, grass.app sounds too general. What is this submodule mainly for? If for the "main GRASS GIS executable" (startup script?), I think grass.startup or grass.init would be more appropriate.

@lindakladivova
Copy link
Contributor

@lindakladivova lindakladivova commented Jan 14, 2021

@lindakladivova @wenzeslaus I think I'm missing something here. What is the rationale behind all this workflow for creating an unrequested location and mapset? If a last used mapset is in an unusable state (not clear what it means... because another user is already using it? Or has GRASS crashed before leaving the mapset in an "unusable" and unfixable state?), shouldn't we try to fix the mapset instead of creating a new default location and mapset silently? What if the default mapset in the default location becomes "unusable"? Create yet another default mapset 2 in that same default location or a default mapset in a new default location 2? And keep creating a new mapset whenever the last used default mapset becomes unusable?

The last step in yellow is also unclear. Does it create a new default location first and then what does it suggest to do in "To continue..."? If the user continues, what happens? If not, then what? Continue to switch to the last used mapset (is it possible now?)? Not continue to start in the just created default mapset?

Other than the workflow, moving code for non-GUI functionalities to grass.app looks fine. BTW, grass.app sounds too general. What is this submodule mainly for? If for the "main GRASS GIS executable" (startup script?), I think grass.startup or grass.init would be more appropriate.

@HuidaeCho Well, we are still not sure how to solve the case when the last mapset is not in the usable state. The notion is that the behavior should be consistent. In this proposal, if the last used mapset is not in the usable state (yes, GRASS crashed or another user is using it) we use the default location (demolocation) as workarounds in order to not use the old startup screen anymore. The GRASS GIS offers the default location and informs the user about that in the Info Bar which has the same form as the one used newly for every first-time user when starting GRASS. The text of the Info Bar is in the proposal in a yellow box. It has a character of advice because the user may be surprised why he is in demolocation, so the Info Bar explains why and advises what to do next.

As you have noted, there can be a case when the default mapset in the default location is in the usable state.. In this case, we can create the user mapset because we need "some" mapset, and show the Info Bar to the user as well.

Let's discuss about it :-) I believe we can come up with a better solution. I also created a second a bit more complex proposal which takes into consideration the last used location.

Proposal 2:
normal_user_diagram2

However, the disadvantage of the second proposal is that the GRASS GIS startup process would not be consistent. The first solution is always consistent - boot in the demolocation in each non-standard situation and provide the same message in the Info Bar. Another thing about the Proposal 2 is that it presents a new concept, which assumes that the PERMANENT mapset is some kind of default mapset. This would mean incorporating this idea into other segments of GRASS. For example, in the Data Catalog context menu, there should be a possibility to switch to location (its PERMANENT) from the location node.

@wenzeslaus
Copy link
Member Author

@wenzeslaus wenzeslaus commented Jan 14, 2021

@lindakladivova Can you please open a new issue for this and restart the discussion there? This PR is about refactoring. Nobody will be looking for this discussion here.

@HuidaeCho
Copy link
Member

@HuidaeCho HuidaeCho commented Jan 14, 2021

@lindakladivova @wenzeslaus My point is when a mapset becomes unusable, we should advise users how to fix it, not deviate them from their intended workflow. If GRASS crashed and the last used mapset became not startable because of some garbage, most of the case, I think we can fix it programmatically. If not (e.g, broken metadata files let's say), ideally, GRASS should be able to detect which part of metadata is broken and ask the user what's missing and fix it.

If someone else is using the last used mapset, we should give the user options with a warning that someone is using the mapset: 1) just don't start GRASS (maybe, with an email notification when the current user leaves, but it would require a whole different discussion and architecture) or 2) let the user create a new mapset in the last used location (because the user will be able to access data in the last used mapset by tweaking the search path) with her/his choice of a mapset name. In any case, when GRASS supports the last used mapset when no mapset is specified, that means we want to put them in the last used mapset or at least in the last used location, not in a world-wide latlong location in any case. That would be a big surprise for me when I assume that I would fall in the last used mapset in a certain projection (location). Starting in a world location may be fine when there are no locations at all, but still I would ask them if they want it or some other projection.

Trying to create a new default location or mapset can be very tricky because, again, we may be trapped in a loop where we have to keep creating a new mapset or location when the previous default mapset becomes unusable. In the second proposal, I would say the PERMANENT mapset is not meant to be a default mapset. I just feel like it's not a working mapset, but it's more for storing original data in a clean state before any analysis in other mapsets. Maybe, it's just me.

Now, are you planning to implement all this workflow only in the GUI startup? Or is it going to be part of the CLI as well to be consistent? Also, are you going to ask the user location or mapset names before creating them?

@HuidaeCho
Copy link
Member

@HuidaeCho HuidaeCho commented Jan 14, 2021

@lindakladivova Can you please open a new issue for this and restart the discussion there? This PR is about refactoring. Nobody will be looking for this discussion here.

I agree. @wenzeslaus About "Opens the old startup when the PERMANENT of demolocation is not usable." When do we start in the PERMANENT of demolocation? The very first time run?

@wenzeslaus
Copy link
Member Author

@wenzeslaus wenzeslaus commented Jan 15, 2021

BTW, grass.app sounds too general. What is this submodule mainly for? If for the "main GRASS GIS executable" (startup script?), I think grass.startup or grass.init would be more appropriate.

Thank you, this is exactly the feedback I was looking for. I considered those, but as you say, it is the main executable. It doesn't just start GRASS and goes away. It does much more. Yes, it is in lib/init because in the past you could almost (or really?) source it because it was really just setting up some variables and calling few commands. There is also the startup in GUI (aka gis_set), but that is/was used only for startup. There is also setup in grass.script. That is really just the initialization and nothing more. On the other hand, grass.py now is the app from user perspective and it does much more than just init. It runs and closes GUI for you, it runs modules and scripts, creates locations and mapsets, manages the temporary ones. Additionally, init is not really great choice in Python (because of __init__, same for main/__main__ and exec) while grass.startup would be strange next to existing grass.script.setup. Perhaps most importantly, grass.app makes it clear that it is for the app (as programs are called nowadays), not necessarily as the best API to use instead of grass.script.setup or now external grass_session.

@HuidaeCho
Copy link
Member

@HuidaeCho HuidaeCho commented Jan 15, 2021

BTW, grass.app sounds too general. What is this submodule mainly for? If for the "main GRASS GIS executable" (startup script?), I think grass.startup or grass.init would be more appropriate.

On the other hand, grass.py now is the app from user perspective and it does much more than just init. It runs and closes GUI for you, it runs modules and scripts, creates locations and mapsets, manages the temporary ones. Additionally, init is not really great choice in Python (because of __init__, same for main/__main__ and exec) while grass.startup would be strange next to existing grass.script.setup. Perhaps most importantly, grass.app makes it clear that it is for the app (as programs are called nowadays), not necessarily as the best API to use instead of grass.script.setup or now external grass_session.

I got it. Personally, I just don't like the recent term "app". Sounds light, too casual, and not serious for scientific software. I thought about alternatives. If only for CLI,

  • grass.console
  • grass.cli

But, grass.py also starts GUI.

@wenzeslaus
Copy link
Member Author

@wenzeslaus wenzeslaus commented Jan 16, 2021

Personally, I just don't like the recent term "app". Sounds light, too casual, and not serious for scientific software.

I agree that it is used in this casual meaning a lot, but it is also a software development term. Simply short for application (Gtk.Application, QApplication), so we have wx.App in wxPython and Qt examples often have QApplication app(argc, argv);. To sound scientific we could call it grass.ggcp for GRASS GIS Computer Program or perhaps we can find a clever acronym. 😄

I thought about alternatives. If only for CLI,

grass.console

I'm not excited about this one. We do have this in GUI for something related, yet different. My feeling is also that the usage of this term is diminishing in favor of shell and terminal. Perhaps most importantly, I'm guessing most people are now familiar with a gaming console.

grass.cli

I actually considered this for slightly different purpose, namely a more reusable set of functions used to build that "application". Ideally, reusable/refactored functions would be moved from grass.app to grass.cli. grass.app would be the code specific for the application. grass.cli would be the main package used to build that application. (There should/could be also grass.gui and grass.app could use both directly if needed or through a script like now.)

But, grass.py also starts GUI.

grass.shell (GUI is also a shell in a broad definition https://en.wikipedia.org/wiki/Shell_(computing)).

Yes, there is the broad definition which seems to be growing in popularity, but we have shell in the more narrow sense playing an important role (our current --text is basically --shell, for example).

grass.ui

It also executes subprocesses without any user interface with --exec, so it is even more general than UI unless we say that it itself provides CLI which counts as UI, but that still would go against using this for programming as an API. Additionally, the package contains/will contain more than just the user interface, it setups the session, etc.

What do you think? Python package is easy to rename even keep backwards compatibility, but I would rather pick the right name right away.

@HuidaeCho
Copy link
Member

@HuidaeCho HuidaeCho commented Jan 16, 2021

Personally, I just don't like the recent term "app". Sounds light, too casual, and not serious for scientific software.

I agree that it is used in this casual meaning a lot, but it is also a software development term. Simply short for application (Gtk.Application, QApplication), so we have wx.App in wxPython and Qt examples often have QApplication app(argc, argv);. To sound scientific we could call it grass.ggcp for GRASS GIS Computer Program or perhaps we can find a clever acronym. smile

I understand "app" is a shortcut for "application", but that script alone is not the entire GRASS application itself either. GRASS is an ecosystem where the init script, C (and some C++) modules, and Python modules live together. Having said that, it's really not that big a deal no matter how we call it. We just try to make it right from the beginning as you said.

I thought about alternatives. If only for CLI,
grass.console

I'm not excited about this one. We do have this in GUI for something related, yet different. My feeling is also that the usage of this term is diminishing in favor of shell and terminal. Perhaps most importantly, I'm guessing most people are now familiar with a gaming console.

I agree. Console is more for terminal, either text or GUI.

grass.cli

I actually considered this for slightly different purpose, namely a more reusable set of functions used to build that "application". Ideally, reusable/refactored functions would be moved from grass.app to grass.cli. grass.app would be the code specific for the application. grass.cli would be the main package used to build that application. (There should/could be also grass.gui and grass.app could use both directly if needed or through a script like now.)

Since grass.app can start GUI, grass.cli is not really a good idea. Do you mean that you want to move code for building terminal or GUI CLI to grass.cli?

But, grass.py also starts GUI.
grass.shell (GUI is also a shell in a broad definition https://en.wikipedia.org/wiki/Shell_(computing)).

Yes, there is the broad definition which seems to be growing in popularity, but we have shell in the more narrow sense playing an important role (our current --text is basically --shell, for example).

I would say, we have --text and --gui shells and grass.py starts one of these shells. From this perspective, grass.shell sounds right.

grass.ui

It also executes subprocesses without any user interface with --exec, so it is even more general than UI unless we say that it itself provides CLI which counts as UI, but that still would go against using this for programming as an API. Additionally, the package contains/will contain more than just the user interface, it setups the session, etc.

What do you think? Python package is easy to rename even keep backwards compatibility, but I would rather pick the right name right away.

I agree. What about "grass.session"? In the end, we're talking about one session of GRASS no matter how we use it (text, gui, exec). Or "grass.shell" is also good, but it could be misleading to just mean the text shell.

@HuidaeCho
Copy link
Member

@HuidaeCho HuidaeCho commented Jan 16, 2021

grass.y? ;-) #1251 (comment)

grass.ing? They actually sound good and witty if all else fails. I'm serious...

grass.ful, grass.ious, grass.ous, grass.cool... I'll stop here. Sorry...

@wenzeslaus
Copy link
Member Author

@wenzeslaus wenzeslaus commented Jan 17, 2021

I understand "app" is a shortcut for "application", but that script alone is not the entire GRASS application itself either.

My point with the app/application really is that the term is used for something which represents the main or top-level object. If you open source code, and you are looking for the thing/layer which is between the application and the system, you know not to look for plugins, libraries, but you are looking for names such as app. That's what I had in mind with app.

I thought about alternatives. If only for CLI,

grass.cli

I actually considered this for slightly different purpose, namely a more reusable set of functions used to build that "application". Ideally, reusable/refactored functions would be moved from grass.app to grass.cli. grass.app would be the code specific for the application. grass.cli would be the main package used to build that application. (There should/could be also grass.gui and grass.app could use both directly if needed or through a script like now.)

Since grass.app can start GUI, grass.cli is not really a good idea. Do you mean that you want to move code for building terminal or GUI CLI to grass.cli?

I'm not sure what exactly you are asking, but any GUI is technically started from CLI, so in that sense it is contained there. Nevertheless, grass.cli looks great to me for the reusable CLI components, perhaps in the future even mixed with some functionality for modules (--exec blurs the command line together already). The concept for grass.app (or whatever) is now more, "whatever grass.py needs is here" rather than anything reusable.

But, grass.py also starts GUI.
grass.shell (GUI is also a shell in a broad definition https://en.wikipedia.org/wiki/Shell_(computing)).

Yes, there is the broad definition which seems to be growing in popularity, but we have shell in the more narrow sense playing an important role (our current --text is basically --shell, for example).

I would say, we have --text and --gui shells and grass.py starts one of these shells. From this perspective, grass.shell sounds right. [...] but it could be misleading to just mean the text shell.

With --exec, or -c -e, it does not start either of these shells, so perhaps even more problematic than ui. I should also qualify that although I think the broad definition is gaining popularity, I don't think most people understand it this way.

What about "grass.session"? In the end, we're talking about one session of GRASS no matter how we use it (text, gui, exec).

That's a nice one, but I think we should reserve it for what is now in the external grass-session. I think that code should be in GRASS itself eventually. Possibly, grass.app would have this as a dependency. Even if this does not happen, it would be confusing.

grass.y grass.ing grass.ful, grass.ious, grass.ous, grass.cool...

I would add also grass.ssarg. Although not without merit, I think they miss an opportunity here to have a name which documents what the thing is used for. Let's keep them in case we need to name some specialized part of the API. (BTW, grass.pygrass is actually a candidate for splitting into two, so...)

@wenzeslaus
Copy link
Member Author

@wenzeslaus wenzeslaus commented Feb 5, 2021

...If you open source code...you know not to look for plugins, libraries, but you are looking for names such as app...

App is good enough for GDAL, see gdal/apps directory. I go with that.

@HuidaeCho
Copy link
Member

@HuidaeCho HuidaeCho commented Feb 5, 2021

...If you open source code...you know not to look for plugins, libraries, but you are looking for names such as app...

App is good enough for GDAL, see gdal/apps directory. I go with that.

Yeah, we found no better name than that. BTW, what does ssarg mean?

@wenzeslaus wenzeslaus merged commit 28876fe into OSGeo:master Feb 18, 2021
22 of 23 checks passed
22 of 23 checks passed
@github-actions
ubuntu-16.04 build
Details
@github-actions
centos:7 build
Details
@github-actions
gnu99 & c++11
Details
@github-actions
GitHub Super Linter
Details
@github-actions
windows-2019 build and tests
Details
@github-actions
flake8-lib-python
Details
@github-actions
ubuntu-18.04 build
Details
@github-actions
gnu99 & c++14
Details
@github-actions
ubuntu-20.04 build
Details
@github-actions
gnu99 & c++17
Details
@github-actions
gnu11 & c++11
Details
@github-actions
gnu11 & c++14
Details
@github-actions
gnu11 & c++17
Details
@github-actions
gnu17 & c++11
Details
@github-actions
gnu17 & c++14
Details
@github-actions
gnu17 & c++17
Details
@github-actions
ubuntu-18.04 tests
Details
@github-actions
flake8-wxgui
Details
@github-actions
ubuntu-20.04 tests ubuntu-20.04 tests
Details
@github-actions
flake8-scripts
Details
@github-actions
flake8-temporal-modules
Details
@travis-ci
Travis CI - Pull Request Build Passed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
New GUI Startup automation moved this from PRs in progress to Done Feb 18, 2021
@wenzeslaus wenzeslaus deleted the wenzeslaus:create-demolocation-in-init branch Feb 18, 2021
@wenzeslaus wenzeslaus added this to In progress in Joint OGC OSGeo ASF Code Sprint 2021 via automation Feb 18, 2021
@wenzeslaus
Copy link
Member Author

@wenzeslaus wenzeslaus commented Feb 18, 2021

Okay, thanks for looking at this. Merged. GIS INIT is becoming an app.

BTW, what does ssarg mean?

Sounds like something an orc would say.

@wenzeslaus wenzeslaus moved this from In progress to Done in Joint OGC OSGeo ASF Code Sprint 2021 Feb 18, 2021
marisn added a commit to marisn/grass that referenced this pull request Mar 22, 2021
* Moves functionality from gis_set.py to grass.py.
* Moves library functions from wxGUI startup to new grass.app package.
* Opens the old startup when the PERMANENT of demolocation is not usable (as before).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants