OSGeo / grass

GRASS GIS - free and open-source geospatial processing engine
https://grass.osgeo.org
Other
852 stars 309 forks source link

[Feat] Implement module level library file injection into python path #2022

Open marisn opened 2 years ago

marisn commented 2 years ago

At the moment if a python script from the scripts/ folder is using more than one file to implement its functionality, there is no easy way how to access those library files from the main module file. There are two modules performing ugly hacks to work around the issue: r.in.wms and wxpyimgview. r.in.wms is a good example of the problem – it stores additional files in etc/r.in.wms and then uses sys.path.insert to access them https://github.com/OSGeo/grass/blob/2f94d78a6622dd3f78d66bb6b01dc6795dcba2b8/scripts/r.in.wms/r.in.wms.py#L238

My proposal is to create a new Makefile variable allowing to automatically place specified python files into a subfolder located in a python path. This would provide two benefits – 1) ease up building modules with multiple python files and 2) add ability to use one module functionality in other module without making another "main" library.

Makefile of r.foo could contain something like: PYTHONLIBFILES = myfunction1 myfunction2 This would result in following directory layout: $GISBASE/etc/python/grass/modlibs/r_foo/myfunction1.py Thus one could do: from grass.modlibs.r_foo import myfunction1 instead of current:

sys.path.insert(1, os.path.join(os.path.dirname(sys.path[0]), "etc", "r.foo"))
import myfunction1

Thoughts? Who knows Makefiles good enough to implement it?

wenzeslaus commented 2 years ago

The direct sys.path.insert should not be used because we have some wrappers. What about these two functions?

I can't say I particularly like them or that they work 100%, but they are used here and there.

My recent approach was to add to path only when needed, i.e., when import fails (and than import whatever you need ignoring the test import which is sub-optimal):

def add_python_modules_to_path_if_needed():
    try:
        import python_package_or_module_name
    except ImportError:
        from grass.script.utils import set_path
        set_path("grass.module.name")

I agree that there should be a standard place to put Python files for each GRASS module/tool, i.e., create a place for a Python package or subpackage associated with each module. Sharing the functionality makes sense to me, too.

Some _grass.xxx.rfoo is a good idea. I don't know if it would work with other packing systems such as pip - thinking smarter addon installations here - so maybe top-level _rfoo or _grass_rfoo is necessary.

However, changing the directory in makefiles is probably not that difficult. Just by doing that, core modules could all use already set path (it would install next to the grass package) avoiding need for adding to sys.path and importing like now (import r_foo or import lib_r_foo). For addons, we could do the same that is to have a common directory which we always add to Python path in the same way as we deal with executables for addons.

In other words, for starters, we could align treating of Python modules (extra .py files) and/or packages (directories with __init__.py) with how we handle executables (scripts or binaries, esp. GRASS modules/tools): Have a common directory where all go and add this directory to path.

A completely different approach would be to install even core GRASS modules written in Python using pip called from a the makefiles. Each GRASS module would be a Python package and the executable piece would be just an extra piece the pip installation is adding.