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

Start in a demo/startup location #868

Merged

Conversation

lindakarlovska
Copy link
Contributor

@lindakarlovska lindakarlovska commented Aug 1, 2020

After creating this directory, follow with another function to create a simple location with some data.

The location demolocation in distribution (GISBASE) can be used for that. If it does not exist and thus cannot be copied, the startup should continue as usual.

I think the name should be startup or startup_location. It is more descriptive than demolocation but still clearly conveying it is not meant for normal use.

The startup mechanism for the first-time user should make use the mapset concept because it will both ensure preserving the data in PERMANENT intact and, at the same time, it will show how to use the mapset concept at least in the relation to the PERMANENT mapset.

When the startup/demo location is created/copied, a new mapset should be created there and used for starting, not the PERMANENT.

It will be only one mapset, but eventually there will be more than one, so probably some name_1, name_2, ... will be needed.

@lindakarlovska
Copy link
Contributor Author

lindakarlovska commented Aug 1, 2020

I have a problem with testing that. Not able to get into the initial state.
While starting GRASS it displays "No GRASS location found in .. Create new location or choose different GRASS GIS directory."
The reason is that the processing in the function SuggestDatabase ends up on the row 513 and does not dive deeper to the function body. Therefore, it does not get a possible database path or does not create new grass database.

Mentors, do you have the same problem? ...
As I remembered when I implemented creating of default database I did not have that kind of problem...

@lindakarlovska lindakarlovska marked this pull request as draft August 1, 2020 08:17
Copy link
Contributor

@mlennert mlennert left a comment

Choose a reason for hiding this comment

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

I have a problem with testing that. Not able to get into the initial state.
While starting GRASS it displays "No GRASS location found in .. Create new location or choose different GRASS GIS directory."
The reason is that the processing in the function SuggestDatabase ends up on the row 513 and does not dive deeper to the function body. Therefore, it does not get a possible database path or does not create new grass database.

For me this was not the reason, but an incorrect usage of copytree(). See my comments in the code. With the suggested change, it seems to work for me.

gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
@mlennert
Copy link
Contributor

mlennert commented Aug 4, 2020 via email

…artup_location_exists changed to get_startup_location.
@lindakarlovska
Copy link
Contributor Author

On 4/08/20 14:08, Linda Kladivova wrote: @.**** commented on this pull request. ------------------------------------------------------------------------ In gui/wxpython/startup/utils.py <#868 (comment)>: > + home = os.path.expanduser("~") + demolocation = os.path.join(home, "grass", "demolocation"), I think it is not the right one. I found on my computer in "computer/usr/lib/grass78" directory. and GISBASE is /home/linwe/grass/dist.x86_64-pc-linux-gnu for me. And also it was added into Debian installation but not Windows one. Am I right?
GISBASE points to the directory where GRASS GIS is installed. In that directory you will find the bin folder with all the module executables, the script folder with the python scripts, the gui folder with the gui stuff, etc. If you only make, but not install this folder is the dist.x86_64-pc-linux-gnu, but if you run 'make install' this directory will be copied to whatever directory was defined during configure (using the --prefix parameter) as the install directory. In your case I would guess that you have two versions: the one installed through the package manager of your distribution (/usr/lib/grass78) and the one in your copy of the source code.

Ok. That makes sense.

@mlennert mlennert linked an issue Aug 4, 2020 that may be closed by this pull request
@mlennert mlennert added this to PRs in progress in New GUI Startup via automation Aug 4, 2020
@mlennert mlennert added the gsoc Reserved for Google Summer of Code student(s) label Aug 4, 2020
Copy link
Contributor

@mlennert mlennert left a comment

Choose a reason for hiding this comment

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

This seems to work nicely now.
Two issues/questions:

  • IIRC, the basic idea was to start up GRASS GIS immediately into this demolocation. Currently, the user gets the startup screen with the demolocation already present. You could try to change the startup in a way to skip that step.

  • The current demolocation contains two maps (mysites and point) which are not useful at all and will probably confuse the user. I will have to check whether these two maps are indispensable in that demolocation. If that is the case (and so they have to stay in the demolocation in GISBASE), it might be best to erase them after the copytree.

gui/wxpython/gis_set.py Show resolved Hide resolved
gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
Co-authored-by: mlennert <mlennert@club.worldonline.be>
@lindakarlovska lindakarlovska added GUI wxGUI related enhancement New feature or request labels Aug 5, 2020
@lindakarlovska lindakarlovska self-assigned this Aug 5, 2020
@lindakarlovska lindakarlovska marked this pull request as ready for review August 5, 2020 11:53
@lindakarlovska lindakarlovska removed the GUI wxGUI related label Aug 5, 2020
gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
gui/wxpython/gis_set.py Outdated Show resolved Hide resolved
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

I have added comments to fix the code you already have and to achieve the main goal of this and that is to start in the demo/startup location (not just create it).

The tests of existing mapset are a good start, but I don't know if to continue in that in this PR. It needs both addition of the lock check, or basically the "can start in mapset" check, and then also another earlier check of the location already being present in the database. This should address the case of starting multiple sessions as a beginner which is a well defined separate issue. (It will have broader impact than that once this functionality gets combined with #767.)

gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
gui/wxpython/gis_set.py Outdated Show resolved Hide resolved
gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
… But there is still an error when generating mapsets.
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

I don't have particular preference how far this should go in catching all the special cases or preparing for this being used as a fallback option for any user. However, I have troubles testing this as there are serious issues with the code, so I think the important part is to get to something which works in the basic cases and the code does not contain any obvious problems. Then it would be easier to decide whether to continue or merge right away.

gui/wxpython/gis_set.py Outdated Show resolved Hide resolved
gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
Copy link
Member

@wenzeslaus wenzeslaus left a comment

Choose a reason for hiding this comment

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

I'm removing the alternative mapset name part as it does not make sense with how the part in gis_set.py is implemented (it runs only when there is not db at all, so the location is always fresh).

gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
gui/wxpython/startup/utils.py Outdated Show resolved Hide resolved
And fix wording of the detail.
Remove the alternative mapset name part as it does not make sense with how the part in gis_set.py is implemented. The function runs only when there is not db at all, so the location is always fresh, hence there is never an existing mapset (locked or unlocked). The implementation is not enough to handle the actual problem of locking anyway or existing startup location.
@wenzeslaus wenzeslaus marked this pull request as ready for review August 20, 2020 02:54
@wenzeslaus wenzeslaus merged commit 8e8a14a into OSGeo:master Aug 20, 2020
New GUI Startup automation moved this from PRs in progress to Done Aug 20, 2020
@neteler neteler added this to the 8.0.0 milestone Dec 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request gsoc Reserved for Google Summer of Code student(s)
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

[Feat] Start in a demo/startup location
4 participants