wxGUI: startup screen removed #1400
Conversation
lindakarlovska
force-pushed
the
wxGUI-remove-startup-screen
branch
from
3cfc92d
to
80ec058
Compare
|
Shouldn't we say something about how to remove all these mapsets that could eventually be created when the user fails to close GRASS properly or opens multiple sessions...? Especially because s/he needs to activate the pencil first to be able to remove mapsets. Just wondering... Maybe it's just my OCD that makes my mind crash when I see all those user, user_1, user_2, etc 😬 |
|
What's not clear to me is where a new mapset is created. In the screenshots, you used the default demo location only. Is a new mapset created in this demo location only, not in the requested location (makes more sense)? Still, I'm sorry, but I really don't understand the rationale behind this idea of "choose or create automatically a mapset every time GRASS runs." We cannot exactly predict the user's intention when they start GRASS. For me, if a mapset was locked and I accidentally started the same mapset, I would like to be kicked out or offered to remove the lock. Must we always start GRASS without user intervention at all, creating a new mapset sometimes, which in most of the case will have to be manually cleaned up because they are more of temporary fixes from my perspective? If it has to be done, please add an option to turn it off. Related issue #1403. |
Yet, I work only with the default location. We discussed it with Anna, and so far it seems to us clearer to always end up the nonstandard GRASS startup in the default location. But it is quite open. As you write "For me, if a mapset was locked and I accidentally started the same mapset, I would like to be kicked out or offered to remove the lock. " - You are now kicked off and in the Infobar there is the button "Switch to last used mapset" and you can switch to it. I know that there is a lack in generating user mapsets, but IMHO I am still convinced that it is a smaller problem compared to keeping and maintaining the user-unfriendly and old startup screen... |
I like this solution, it will avoid cluttering the location with empty (?) |
|
@HuidaeCho @veroandreo About all this cluttering with empty mapsets, how often do you think it happens? The current code in the PR tries PERMANENT mapset and steps through other candidate (mapset) names till it hits one which is usable. If it is the PERMANENT mapset, no mapset is created. If one of these candidates is usable, it uses that and no other mapset is created, etc. As far as I understand the code, the cluttering happens when you keep starting new sessions. Is this a realistic use case which we need to specifically address in the design? Note also that the locking part is at this point irrelevant for Windows where the locking is not enabled (I don't know why).
@HuidaeCho If turned of, the question would be what happens instead. For example, making location/mapset optional is an interesting way to address this issue, but the question is how to implement that.
@lindakladivova I consider this nice to have, but not necessary. Depends on how hard this would be to implement. You started to heavily leverage the "gisrc" file, similarly to that, perhaps the VAR file in the mapset is a place for marking mapset as temporary. The concept of a temporary mapset is already there ( |
It just happened to me 10 minutes ago. I already opened the NC sample mapset in one desktop and tried to open it again in another, all from the same computer. With this PR, I would have ended up in the world demo location, which has nothing to do with the NC sample. I would be lucky that I won't have to delete a new temporary mapset because the default demo mapset was not locked. In some sense, it's better to create all these temporary mapsets only in the demo location to avoid cluttering user locations. Cleaning would be easier. So this PR is mainly for those who start GRASS using the icon, but I start it from the terminal. I think I found a solution. If I add
If it's usable because GUI is in the same session, that will be great. This solution will allow a delayed GUI startup only after the CLI session started successfully. |
You were not lucky. This is how it should behave. No mapset was (would be) created, so no clutter is accumulating which is exactly my point: The scenario when you end up with location cluttered by these mapsets is unlikely.
Right, it is a compromise. We could create completely temporary location, but that brings in a lot of other issues like what should happen when a user starts to import data there. Similar, although smaller, issue is with the temporary mapset.
Yes, something like that. And that's the whole issue here. GRASS power user starting from command line can probably deal with GUI (and GRASS itself) not starting at all in case mapset is locked. However, we are trying to have find a solution which works for all users who use GUI and not just when the mapset is locked. We need to provide something like data catalog/startup screen which allows all kinds of management (force unlock, all database, ..., view content) which leads us to delivering the Data tab which needs an active session, so we need to pick a reasonable mapset.
Well, we know when we are running in interactive terminal, so we could behave differently in that case. However, Windows users seem to insist on CLI (#1221), so this distinction could be applied only on Linux and maybe macOS for the time being and that assumes that #1221 gets merged and actually applied in distributions/installations.
Yes, at least that part should work. GUI behaves like any other GRASS GIS module (which sounds nice on the surface, but it is partially the source of this whole starting GRASS issue; if we change that, we have potentially more options available to us). |
Thanks much for this explanation! I have not understood this before and hence my concern. But like this, to get the location cluttered with |
@wenzeslaus I also think that it is not necessary. I realize that there is quite a heavy change in gisrc :-). I was thinking about how to code this PR very long time and finally, used gisrc file because the last used mapset, location, grassdb here are closely related to this issue, so everything would be at the same place. And also reading gisrc in catalog.py is quite elegant. What do you think about the concept I came up with? |
|
@lindakladivova Please make the location of the default database ( |
@HuidaeCho I like your note about extending the Maybe the default location is a bit more correct collocation here than demolocation (note that default location is also used in Info Bar) since we provide a common EPSG: 4326 option which a user can immediately start using. |
I agree with you. It all depends on how we look at this database. Is it a demo (probably could be hidden in
Absolutely, the |
Today and in the last 10+ years we used and use network drives for storing "grassdata/" (in IRST, in FEM, in some HPC infrastructure, in mundialis, ...). Hence it would be generally important for us to avoid any hardcoded paths. |
This doesn't make much sense to me, rc is not a place users should be editing. Why not create it in current database? Disadvantage is that if you use multiple databases, it would eventually be created in each. Alternatively, we could create a temporary empty location, but that would be probably little more complex, e.g. maybe we would want to hide it from data catalog and when user would switch to other location, it would need to be removed. I think we wouldn't have issues with user importing data into it as long as we clearly inform them in the infobar. |
|
Let me comment on what I think the rc (
|
Not necessarily directly. Users can use Alternatively, an environment variable could be another option. It would be more permanent...
That will be a big disadvantage.
Only if we can guarantee a clean removal of the temp location. e.g., multiple sessions in the same temp location and exit one session at a time? Or temp location per session? |
I think that concept of temporary location will be again something confusing for a first-time user. There is still a risk that a user would not read the advice in the infobar... and then he could lose all the data. |
Yes that's what I meant, the temporary one would be only for these cases, when you can't use the last used mapset.
I wouldn't be worried about that too much. The infobar is hard to miss and even if you ignore it, you don't have any data there, which should tell you something is wrong. GUI could be aware that it is running in a temporary location (e.g. based on the name) so we could use that in some way to additionally warn users when they try to import data etc. I don't have this fully thought through, there may be some associated problems with this, but I think it's worth looking into this direction. |
The idea is to not run multiple sessions in one temp location, there could be multiple at the same time. |
…ntMapsetChanged connects infobar dismissing
lindakarlovska
force-pushed
the
wxGUI-remove-startup-screen
branch
from
a352e87
to
f79f263
Compare
So we would prefer the failing as before or you think it is correct to jump to tmploc? Maybe, we should have the same behavior everywhere... |
Let's discuss this in a separate issue if it seems like needed after merging this PR. Current but read-only would be a new concept we don't have any mechanism for. |
Just to record here what we discussed elsewhere: There are different expectation for the CLI behavior than for GUI, so it makes sense here to keep the current |
… now controls if mapset is valid.
Yep, clear. I have corrected the behavior in this situation. |
…ries of checks and setup). Do not rely on some later function to do the checking, i.e., remove the additional check in lock_mapset.
There was a problem hiding this comment.
I agree with the code after my fixes to checking behavior. --text with broken mapset now gives the detailed message with reasons as it does with explicit mapset in command line, not just mapset invalid. For consistency, safety, and possibly future all paths now lead to a set_mapset call.
The code needs refactoring more than it did before and first time user gets the WGS84 location even in the command line. I suggest that this and any other issues are left for new issues/PRs.
|
Merged, but there is couple of loose ends which will need to be addressed in subsequent PRs, most importantly what to do with |
* Startup screen code removed. Used only when * Either demo/default world location is used (first time user) or an XY location temporary location is used (fallback). * Last mapset path is now noted in gisrc of the current session. Used in GUI to offer switch from fallback to locked. * non_standard_startup and first_time_user in grassdb/checks.py. * grassdb/config for special strings such as `<UNKNOWN>`. * Fallback tmp location only for GUI. Default currently for both GUI and CLI (`--text`). * Call set_mapset in every branch (all paths lead to the same series of checks and setup). * This increases the need for refactoring of `main()` and needs removal of `--gtext`.
Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>
* Startup screen code removed. Used only when * Either demo/default world location is used (first time user) or an XY location temporary location is used (fallback). * Last mapset path is now noted in gisrc of the current session. Used in GUI to offer switch from fallback to locked. * non_standard_startup and first_time_user in grassdb/checks.py. * grassdb/config for special strings such as `<UNKNOWN>`. * Fallback tmp location only for GUI. Default currently for both GUI and CLI (`--text`). * Call set_mapset in every branch (all paths lead to the same series of checks and setup). * This increases the need for refactoring of `main()` and needs removal of `--gtext`.
Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>
Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>
Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>
STATE BEFORE THIS PR:
Before this PR, when the last mapset was not usable, the startup screen still appeared.
EXPECTED STATE AFTER THIS PR:
This PR suggests the entire removal of the startup screen and using the default location EPSG:4326 with Data Catalog for the user's data hierarchy definition.
It uses Infobar that informs the user about the reason why they are in the demolocation and suggests further process - creating or finding another usable mapset through Data Catalog.
To be clearer, I will explain here which situations can occur and how the new startup copes with them.
First-time startup
Still the same as before this PR. The data structure info is given to the user.
Second GRASS GUI startup (without closing the first session)
GRASS was not closed, therefore, the user is still considered a beginner. No settings were saved to the RC file. Whereas the PERMANENT mapset is used, the startup creates a spare mapset called after a user.
Third GRASS GUI startup (without closing the second session)
Whereas the user mapset is used, the startup creates a spare mapset called "user_1"...
We closed all three sessions.
Then we start up GRASS again:
Last used mapset usable
The normal situation in which the last used mapset in RC file is usable (most common)
Now we will try to start GRASS again without closing the previous session:
Last used mapset is locked
Yes. The last used mapset is locked (running in the previous session). The infobar is telling us the option for switching and removing gislock or we can set usable mapset through Data Catalog.
To complete the list of situations, two other situations can occur:
Last usable mapset is invalid
Last usable mapset is owned by different user
Here, GRASS chooses PERMANENT mapset, if not capable, then "user" mapset, if "user" mapset not capable, then it chooses or creates user_2 mapset and so on...
In all situations, the default location is used with applicable advice provided in Infobar.
All comments welcome! :-) This is very important PR so the more discussion the better!