move everything from /lib/init/grass.py to /lib/python/init

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
7 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

move everything from /lib/init/grass.py to /lib/python/init

Pietro Zambelli
Dear devs,

What do you think if we move all the function that at the moment are contained in `/lib/init/grass.py` into a new subfolder under `/lib/python`?

The main advantages that I see are:

- start a python script in GRASS just setting the the python path and then I can use the same functions that are defined for the normal GRASS start up, without duplicate code;
- We can add unittest for the start up functions
- We can remove code duplication between grass.init and grass.script.core


What do you think?

I did the changes and at least on my computer GRASS is working, all the changes are available at this link:

https://git.osgeo.org/gogs/zarch/grass/commit/27c8351423da645d938fb6c2e54781ee24e6f074


I've split the functions that were contained in the grass.py in the following files, any comments?


```
$ rg -e "^def\s[a-z_]+\(|^class\s[A-Z][a-z]*|^[A-Za-z_]+\s*="
      
gettext.py
11:_ = gettext.gettext

message.py
7:_DEBUG = None
10:def warning(text):
14:def fatal(msg):
19:def message(msg):
24:def is_debug():
41:def debug(msg):

utils.py
13:def grep(pattern, lines):
23:def print_params():
62:def get_username():
85:def make_fontcap():
93:def ensure_db_connected(mapset):
102:def get_shell():

gui.py
20:def read_gui(gisrc, default_gui):
53:def check_gui(expected_gui):
94:def save_gui(gisrc, grass_gui):
102:def gui_startup(grass_gui):
135:def start_gui(grass_gui):
148:def close_gui():
165:def clear_screen():
176:def show_banner():
188:def say_hello():
203:def show_info(shellname, grass_gui, default_gui):
229:def csh_startup(location, location_name, mapset, grass_env_file):
280:def bash_startup(location, location_name, grass_env_file):
313:PROMPT_COMMAND=grass_prompt\n""" % (_("2D and 3D raster MASKs present"),
338:def default_startup(location, location_name):
353:def done_message():

subprocess.py

10:def call(cmd, **kwargs):

info.py
5:BUILD_GISBASE = "@GISBASE@"
6:BUILD_PROJSHARE = "@CONFIG_PROJSHARE@"
7:CMD_NAME = "@START_UP@"
8:GRASS_VERSION = "@GRASS_VERSION_NUMBER@"
9:LD_LIBRARY_PATH = '@LD_LIBRARY_PATH_VAR@'
12:GISBASE = os.path.normpath(os.environ.get("GISBASE", BUILD_GISBASE))
13:GRASS_PROJSHARE = os.environ.get("GRASS_PROJSHARE", BUILD_PROJSHARE)

data.py
13:def create_location(gisdbase, location, geostring):
47:def is_mapset_valid(full_mapset):
56:def is_location_valid(gisdbase, location):
72:def get_mapset_invalid_reason(gisdbase, location, mapset):
106:def set_mapset(gisrc, arg, geofile=None, create_new=False):
191:def set_mapset_interactive(grass_gui):
218:def lock_mapset(mapset_path, force_gislock_removal, user, grass_gui):
270:class MapsetSettings(object):
301:def load_gisrc(gisrc, gisrcrc):

clean.py
8:def try_remove(path):
15:def try_rmdir(path):
22:def cleanup_dir(path):
33:class Cleaner(object):  # pylint: disable=R0903

env.py
22:def path_prepend(directory, var):
31:def path_append(directory, var):
40:def set_paths(grass_config_dir):
97:def set_defaults():
126:def set_display_defaults():
134:def set_browser():
173:def ensure_home():
180:def clean_env(gisrc):
192:def load_env(grass_env_file):
218:def set_language(grass_config_dir):

compatibility.py
5:ENCODING = locale.getdefaultlocale()[1]
11:def to_text_string(obj, encoding=ENCODING):

path.py
11:_WXPYTHON_BASE = None
14:def readfile(path):
21:def writefile(path, s):
27:def gpath(*args):
35:def wxpath(*args):
50:def get_grass_config_dir():
78:def create_tmp(user, gis_lock):
125:def clean_temp():
132:def get_gisrc_from_config_dir(grass_config_dir, batch_job):
143:def get_grass_env_file(sh, grass_config_dir):
160:def find_exe(pgm):

system.py
4:windows = sys.platform == 'win32'
5:cygwin = "cygwin" in sys.platform
6:macosx = "darwin" in sys.platform

gisrc.py
13:def create_gisrc(tmpdir, gisrcrc):
33:def read_gisrc(filename):
54:def read_env_file(path):
64:def write_gisrc(kv, filename):
71:def create_initial_gisrc(filename):

batch.py
9:def get_batch_job_from_env_variable():
32:def run_batch_job(batch_job):

parser.py
29:help_text = r"""GRASS GIS $VERSION_NUMBER
114:def help_message(default_gui):
121:class Parameters(object):
135:def parse_cmdline(argv, default_gui):
183:def main(argv):

```

I wish you all a nice week-end.

Pietro

_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: move everything from /lib/init/grass.py to /lib/python/init

wenzeslaus
Hi, Pietro. Just a short answer for now. This is exactly what I had in my mind when doing the last major changes in the grass.py file. I generally like the layout you suggested. It seems to me that choosing a good name for the whole module will be a bit tricky. Also I think one reason for having them there was that grass.py works without a he G Python lib found. Vaclav

On Jul 14, 2017 10:00 AM, "Pietro" <[hidden email]> wrote:
Dear devs,

What do you think if we move all the function that at the moment are contained in `/lib/init/grass.py` into a new subfolder under `/lib/python`?

The main advantages that I see are:

- start a python script in GRASS just setting the the python path and then I can use the same functions that are defined for the normal GRASS start up, without duplicate code;
- We can add unittest for the start up functions
- We can remove code duplication between grass.init and grass.script.core


What do you think?

I did the changes and at least on my computer GRASS is working, all the changes are available at this link:

https://git.osgeo.org/gogs/zarch/grass/commit/27c8351423da645d938fb6c2e54781ee24e6f074


I've split the functions that were contained in the grass.py in the following files, any comments?


```
$ rg -e "^def\s[a-z_]+\(|^class\s[A-Z][a-z]*|^[A-Za-z_]+\s*="
      
gettext.py
11:_ = gettext.gettext

message.py
7:_DEBUG = None
10:def warning(text):
14:def fatal(msg):
19:def message(msg):
24:def is_debug():
41:def debug(msg):

utils.py
13:def grep(pattern, lines):
23:def print_params():
62:def get_username():
85:def make_fontcap():
93:def ensure_db_connected(mapset):
102:def get_shell():

gui.py
20:def read_gui(gisrc, default_gui):
53:def check_gui(expected_gui):
94:def save_gui(gisrc, grass_gui):
102:def gui_startup(grass_gui):
135:def start_gui(grass_gui):
148:def close_gui():
165:def clear_screen():
176:def show_banner():
188:def say_hello():
203:def show_info(shellname, grass_gui, default_gui):
229:def csh_startup(location, location_name, mapset, grass_env_file):
280:def bash_startup(location, location_name, grass_env_file):
313:PROMPT_COMMAND=grass_prompt\n""" % (_("2D and 3D raster MASKs present"),
338:def default_startup(location, location_name):
353:def done_message():

subprocess.py

10:def call(cmd, **kwargs):

info.py
5:BUILD_GISBASE = "@GISBASE@"
6:BUILD_PROJSHARE = "@CONFIG_PROJSHARE@"
7:CMD_NAME = "@START_UP@"
8:GRASS_VERSION = "@GRASS_VERSION_NUMBER@"
9:LD_LIBRARY_PATH = '@LD_LIBRARY_PATH_VAR@'
12:GISBASE = os.path.normpath(os.environ.get("GISBASE", BUILD_GISBASE))
13:GRASS_PROJSHARE = os.environ.get("GRASS_PROJSHARE", BUILD_PROJSHARE)

data.py
13:def create_location(gisdbase, location, geostring):
47:def is_mapset_valid(full_mapset):
56:def is_location_valid(gisdbase, location):
72:def get_mapset_invalid_reason(gisdbase, location, mapset):
106:def set_mapset(gisrc, arg, geofile=None, create_new=False):
191:def set_mapset_interactive(grass_gui):
218:def lock_mapset(mapset_path, force_gislock_removal, user, grass_gui):
270:class MapsetSettings(object):
301:def load_gisrc(gisrc, gisrcrc):

clean.py
8:def try_remove(path):
15:def try_rmdir(path):
22:def cleanup_dir(path):
33:class Cleaner(object):  # pylint: disable=R0903

env.py
22:def path_prepend(directory, var):
31:def path_append(directory, var):
40:def set_paths(grass_config_dir):
97:def set_defaults():
126:def set_display_defaults():
134:def set_browser():
173:def ensure_home():
180:def clean_env(gisrc):
192:def load_env(grass_env_file):
218:def set_language(grass_config_dir):

compatibility.py
5:ENCODING = locale.getdefaultlocale()[1]
11:def to_text_string(obj, encoding=ENCODING):

path.py
11:_WXPYTHON_BASE = None
14:def readfile(path):
21:def writefile(path, s):
27:def gpath(*args):
35:def wxpath(*args):
50:def get_grass_config_dir():
78:def create_tmp(user, gis_lock):
125:def clean_temp():
132:def get_gisrc_from_config_dir(grass_config_dir, batch_job):
143:def get_grass_env_file(sh, grass_config_dir):
160:def find_exe(pgm):

system.py
4:windows = sys.platform == 'win32'
5:cygwin = "cygwin" in sys.platform
6:macosx = "darwin" in sys.platform

gisrc.py
13:def create_gisrc(tmpdir, gisrcrc):
33:def read_gisrc(filename):
54:def read_env_file(path):
64:def write_gisrc(kv, filename):
71:def create_initial_gisrc(filename):

batch.py
9:def get_batch_job_from_env_variable():
32:def run_batch_job(batch_job):

parser.py
29:help_text = r"""GRASS GIS $VERSION_NUMBER
114:def help_message(default_gui):
121:class Parameters(object):
135:def parse_cmdline(argv, default_gui):
183:def main(argv):

```

I wish you all a nice week-end.

Pietro

_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev

_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: move everything from /lib/init/grass.py to /lib/python/init

Maris Nartiss
2017-07-14 19:00 GMT+03:00 Vaclav Petras <[hidden email]>:
> Also I think one reason for having
> them there was that grass.py works without a he G Python lib found. Vaclav

This! Although having a module would be fine, we must take extra care
to put warnings in all files to not depend on any other GRASS GIS
functions that might not be available/useable before full
initialisation is done.

Māris.
_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: move everything from /lib/init/grass.py to /lib/python/init

Pietro Zambelli
In reply to this post by wenzeslaus
Hi Vaclav,

On Fri, Jul 14, 2017 at 6:00 PM, Vaclav Petras <[hidden email]> wrote:
This is exactly what I had in my mind when doing the last major changes in the grass.py file.
I generally like the layout you suggested. It seems to me that choosing a good name for the whole module will be a bit tricky.

This is intended as a proof of concept to see the feasibility.
I've try to found a better name but didn't come up to my mind...
Perhaps: session instead of init?

My final objective is to be able to do something like:

```python
import os
import sys

# Perhaps in GRASS8 we will be able to skip this! ;-)
sys.append(os.environ.get('GISBASE', '/home/pietro/my/gisbase'))

from grass.init import Session

# open - close mode
session = Session('mygisdbase/location/mapset')
session.open()
# do my stuff here...
session.close()

# with statement
with Session('mygisdbase/location/mapset') as session:
    # do my stuff here
```

 

Also I think one reason for having them there was that grass.py works without a the G Python lib found.

ok, but I don't see any advantage to have grass.py that works without loading the grass python libraries. and with the current status we have code duplication, that mean more code to read, test and maintain.
If you are afraid that the grass python libraries might be broken and therefore the user can have troubles, they will have troubles in any case (no gui, several important modules missing). So I will consider stopping the user earlier.as a feature.

Let me know.

Pietro

 

_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: move everything from /lib/init/grass.py to /lib/python/init

wenzeslaus


On Mon, Jul 17, 2017 at 12:36 AM, Pietro <[hidden email]> wrote:

On Fri, Jul 14, 2017 at 6:00 PM, Vaclav Petras <[hidden email]> wrote:
This is exactly what I had in my mind when doing the last major changes in the grass.py file.
I generally like the layout you suggested. It seems to me that choosing a good name for the whole module will be a bit tricky.

This is intended as a proof of concept to see the feasibility.
I've try to found a better name but didn't come up to my mind...
Perhaps: session instead of init?

My final objective is to be able to do something like:

That makes sense. In fact, that's very similar to a file I drafted some time ago splitting the data initialization and runtime in grass.script.setup and adding Session (see the attached file). Another example, for a different case, is here:

https://github.com/wenzeslaus/g.remote/blob/master/grasssession.py

# Perhaps in GRASS8 we will be able to skip this! ;-)
sys.append(os.environ.get('GISBASE', '/home/pietro/my/gisbase'))

Added to the list, but how to do it remains unclear (many different discussions in Trac and ML):

from grass.init import Session

# open - close mode
session = Session('mygisdbase/location/mapset')
session.open()
# do my stuff here...
session.close()

# with statement
with Session('mygisdbase/location/mapset') as session:
    # do my stuff here
```

 

Unfortunately, here is where the trouble begins. The above leads to the following:

with Session as session:
    session.run_command(...)

which fits with API which already exists for Ruby:

https://github.com/jgoizueta/grassgis/

GrassGis.session configuration do+
    r.info 'slope'
    g.region '-p'
end

The trouble is that session (at least in Python) needs to depend on the rest of the library because it is the interface for the rest (on demand imports may help here).

So perhaps having grass.init or grass.setup with the low level functions and then a separate grass.session with a nice interface which may depend on all other modules may be a better way. (Although having each function from the library as a method of Session calls for more thinking about grass.session.Session.

Just to be clear: I definitively think this should be done. I'm just not sure what is the right way.

Vaclav

_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev

__init__.py (22K) Download Attachment
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: move everything from /lib/init/grass.py to /lib/python/init

Pietro Zambelli
Hi Vaclav,

sorry for the delay but in the last day I was off-line.

On Mon, Jul 17, 2017 at 5:36 PM, Vaclav Petras <[hidden email]> wrote:

On Mon, Jul 17, 2017 at 12:36 AM, Pietro <[hidden email]> wrote:

On Fri, Jul 14, 2017 at 6:00 PM, Vaclav Petras <[hidden email]> wrote:
This is exactly what I had in my mind when doing the last major changes in the grass.py file.
I generally like the layout you suggested. It seems to me that choosing a good name for the whole module will be a bit tricky.

This is intended as a proof of concept to see the feasibility.
I've try to found a better name but didn't come up to my mind...
Perhaps: session instead of init?

My final objective is to be able to do something like:

That makes sense. In fact, that's very similar to a file I drafted some time ago splitting the data initialization and runtime in grass.script.setup and adding Session (see the attached file). Another example, for a different case, is here:

https://github.com/wenzeslaus/g.remote/blob/master/grasssession.py

Nice module, I was not aware of it!
However I think that the purpose is slightly different. The grassession aims is to generate the session remotely, here I would like to setup a local session. It is true that I should be able to just connect through ssh to the localhost... but it seems to me not the right way. So I've sketch a possible implementation just to see how it could work, see:

https://git.osgeo.org/gogs/zarch/grass/commit/b9cb69a1d7381924b0c2229ba43f21b1b7473c72

 


# Perhaps in GRASS8 we will be able to skip this! ;-)
sys.append(os.environ.get('GISBASE', '/home/pietro/my/gisbase'))

Added to the list, but how to do it remains unclear (many different discussions in Trac and ML):

Thank you
 

from grass.init import Session

# with statement
with Session('mygisdbase/location/mapset') as session:
    # do my stuff here
```

 

Unfortunately, here is where the trouble begins. The above leads to the following:

with Session as session:
    session.run_command(...)

which fits with API which already exists for Ruby:

https://github.com/jgoizueta/grassgis/

GrassGis.session configuration do+
    r.info 'slope'
    g.region '-p'
end

The trouble is that session (at least in Python) needs to depend on the rest of the library because it is the interface for the rest (on demand imports may help here).

Sorry I'm not sure that I get your point here, what do you mean?
The following code is running at the moment on my machine:

```python
import os
import sys

MAPSET = '/home/pietro/docdat/gis/nc_basic_spm_grass7/user1/'
GISBASE = '/home/pietro/docdat/src/gis/ggrass/dist.x86_64-pc-linux-gnu/'

# set the path
sys.path.append(os.path.join(os.environ.get('GISBASE', GISBASE),
                             'etc', 'python'))

# import the python GRASS libraries
from grass.script.core import run_command
from session import Session


with Session(MAPSET) as session:
    run_command('r.slope.aspect', elevation='elevation',
                slope='slope', aspect='aspect',
                overwrite=True)

```



So perhaps having grass.init or grass.setup with the low level functions and then a separate grass.session with a nice interface which may depend on all other modules may be a better way. (Although having each function from the library as a method of Session calls for more thinking about grass.session.Session.

I was thinking to add this Session class definition inside init/session.py to then try to refactor the main function in parser.py:

https://git.osgeo.org/gogs/zarch/grass/src/grass_session/lib/python/init/parser.py#L189

to start a session and then execute all the rest of the functions to start/use the grass shell/gui.
 

Just to be clear: I definitively think this should be done. I'm just not sure what is the right way.

I'm not sure too! This is why I'm trying to sketch some code drafting to understand what is feasible and how should this organized.
Thank you for taking time to review the code/changes and to give me feedback.

Cheers

Pietro


_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: move everything from /lib/init/grass.py to /lib/python/init

wenzeslaus


On Wed, Jul 19, 2017 at 1:42 AM, Pietro <[hidden email]> wrote:
On Mon, Jul 17, 2017 at 5:36 PM, Vaclav Petras <[hidden email]> wrote:

On Mon, Jul 17, 2017 at 12:36 AM, Pietro <[hidden email]> wrote:

On Fri, Jul 14, 2017 at 6:00 PM, Vaclav Petras <[hidden email]> wrote:
This is exactly what I had in my mind when doing the last major changes in the grass.py file.
I generally like the layout you suggested. It seems to me that choosing a good name for the whole module will be a bit tricky.

This is intended as a proof of concept to see the feasibility.
I've try to found a better name but didn't come up to my mind...
Perhaps: session instead of init?

My final objective is to be able to do something like:

That makes sense. In fact, that's very similar to a file I drafted some time ago splitting the data initialization and runtime in grass.script.setup and adding Session (see the attached file). Another example, for a different case, is here:

https://github.com/wenzeslaus/g.remote/blob/master/grasssession.py

Nice module, I was not aware of it!
However I think that the purpose is slightly different. The grassession aims is to generate the session remotely, here I would like to setup a local session. It is true that I should be able to just connect through ssh to the localhost... but it seems to me not the right way.

Sure, the code is for use remote session on another computer, although by switching the backend you can use the API for local session (without any ssh to localhost):
What I'm bring up here is the idea of a Session API which works the same for local session, remote session, or multiple sessions (in one script).
 
So I've sketch a possible implementation just to see how it could work, see:

https://git.osgeo.org/gogs/zarch/grass/commit/b9cb69a1d7381924b0c2229ba43f21b1b7473c72


Looks nice and clean. The difference to the above is that it does not contain API for actually executing anything which removes the dependency problems I mentioned earlier. This is probably much more fitting to the current session as opposed to the remote (or generally "other") session as used in g.remote. In other words, we have two different concepts for a Session object (API): SessionA which sets the global state and thus sets up a current session and SessionB which sets up a session which is remote or local (but does touch the global state, i.e. the current session). SessionA does not have any extra functions and all is executed through standard (current) APIs while SessionB needs to provide all (or at least some) functions for execution of modules or code (e.g. g.remote executes scripts).

Besides the fact that SessionA and SessionB have very different APIs and behavior, my concern is that I think we should consider (and possibly cover) the use case of parallel processing in different mapsets or parallel rendering. SessionA is not good for this. SessionB is.

Another concern is the usage of context manager. It makes sense. It is a resource, you connect, you open. But the state which is changes is global. Is that expected from context manager? I don't know, I need to read the PEP.


from grass.init import Session

# with statement
with Session('mygisdbase/location/mapset') as session:
    # do my stuff here
```

 

Unfortunately, here is where the trouble begins. The above leads to the following:

with Session as session:
    session.run_command(...)

which fits with API which already exists for Ruby:

https://github.com/jgoizueta/grassgis/

GrassGis.session configuration do+
    r.info 'slope'
    g.region '-p'
end

The trouble is that session (at least in Python) needs to depend on the rest of the library because it is the interface for the rest (on demand imports may help here).

Sorry I'm not sure that I get your point here, what do you mean?
The following code is running at the moment on my machine:

```python
import os
import sys

MAPSET = '/home/pietro/docdat/gis/nc_basic_spm_grass7/user1/'
GISBASE = '/home/pietro/docdat/src/gis/ggrass/dist.x86_64-pc-linux-gnu/'

# set the path
sys.path.append(os.path.join(os.environ.get('GISBASE', GISBASE),
                             'etc', 'python'))

# import the python GRASS libraries
from grass.script.core import run_command
from session import Session


with Session(MAPSET) as session:
    run_command('r.slope.aspect', elevation='elevation',
                slope='slope', aspect='aspect',
                overwrite=True)

```



My concern is just another concern about the context manager. Is is OK that it is actually not used in the with block? Again, that's something PEP hopefully answers.

Whatever would be the default usage, I can see how it could be used. Here is an example for the use case when we are currently passing the env parameter (like the parallel processing mentioned above):

```
with Session(MAPSET) as session:
    run_command('r.slope.aspect', elevation='elevation',
                slope='slope', aspect='aspect',
                overwrite=True, env=session.env)
```
 

So perhaps having grass.init or grass.setup with the low level functions and then a separate grass.session with a nice interface which may depend on all other modules may be a better way. (Although having each function from the library as a method of Session calls for more thinking about grass.session.Session.

I was thinking to add this Session class definition inside init/session.py to then try to refactor the main function in parser.py:

https://git.osgeo.org/gogs/zarch/grass/src/grass_session/lib/python/init/parser.py#L189

to start a session and then execute all the rest of the functions to start/use the grass shell/gui.
 

I'm not sure what are the changes you plan. What I can say now is that I suggest to not use the name parser.py as it means something very specific in GRASS. I'm also not sure if main() of lib/init/grass.py should be part of the library (it seems to me actually impossible as it should be on the executable path rather than a library path).

_______________________________________________
grass-dev mailing list
[hidden email]
https://lists.osgeo.org/mailman/listinfo/grass-dev
Loading...