PyGRASS grid calls g.region at import time

[huhabla]

There is an issue in pygrass that prevent the compilation of the temporal framework on windows[1,2]. The issue appears while importing grass.pygrass.modules.grid.grid. While import a new Module instance is created for g.region.

IMHO its not a good decision to call a grass module directly on import. I would suggest to create a Module variable in GridModule that is only once created at the construction of the first GridModule object in a process (singleton).

[1] ​http://wingrass.fsv.cvut.cz/grass70/logs/log-r57559-690/error.log [2]

Traceback (most recent call last):
  File "c:/osgeo4w/usr/src/grass_trunk/dist.i686-pc-mingw32/scripts/t.remove.py", line 58, in <module>
    import grass.pygrass.modules as pyg
  File "c:\osgeo4w\usr\src\grass_trunk\dist.i686-pc-mingw32\etc\python\grass\pygrass\__init__.py", line 17, in <module>
    import modules
  File "c:\osgeo4w\usr\src\grass_trunk\dist.i686-pc-mingw32\etc\python\grass\pygrass\modules\__init__.py", line 11, in <module>
    import grid
  File "c:\osgeo4w\usr\src\grass_trunk\dist.i686-pc-mingw32\etc\python\grass\pygrass\modules\grid\__init__.py", line 9, in <module>
    import grid
  File "c:\osgeo4w\usr\src\grass_trunk\dist.i686-pc-mingw32\etc\python\grass\pygrass\modules\grid\grid.py", line 23, in <module>
    _GREG = Module('g.region')
  File "c:\osgeo4w\usr\src\grass_trunk\dist.i686-pc-mingw32\etc\python\grass\pygrass\modules\interface\module.py", line 244, in __init__
    raise GrassError(str_err % self.name)
grass.pygrass.errors.GrassError: "Module 'g.region' not found, please check that the module exist"
make[3]: *** [t.remove.tmp.html] Error 1

[martinl]

Anybody working on this issue?

[hamish]

fwiw,

trunk/lib/python/pygrass/modules/grid/grid.py line 23 from the traceback has:

  _GREG = Module('g.region')

which is used within cmd_exe(), to "Create a mapset, and execute a cmd inside."

there are a number of other Module() aliases created within, but no other at the top level of the file, all the others are within a function definition. (but if it stops on the first error maybe that's irrelevant)

The Module() class is defined in trunk/lib/python/pygrass/modules/interface/module.py, the line there where the above failure is caught (L244) is:

    def __init__(self, cmd, *args, **kargs):
        self.name = cmd
        try:
            # call the command with --interface-description
            get_cmd_xml = subprocess.Popen([cmd, "--interface-description"],
                                           stdout=subprocess.PIPE)
        except OSError:
            str_err = "Module %r not found, please check that the module exist"
            raise GrassError(str_err % self.name)

so apparently during the build anything calling that initialization function would want to be run in a dummy session, as is done to generate the header usage text for the html help pages. or the pygrass code change to simply skip past the step with a warning message if GISBASE is not set to anything.

Hamish

[zarch]

Sorry for the late replay but I was in holiday.

As pointed out by Hamish, when we instantiate a Module object with:

Module('g.region')

We are only run the module with the --interface-description and then parsing the xml, I want to avoid to do these operation inside a cycle. Now should be fix.

[glynn]

Replying to zarch:

We are only run the module with the --interface-description and then parsing the xml

Even that is too work on import.

An import should only execute function and class definitions, and variable assignments where the value is "simple": no potentially-large loops, no executing external commands, preferably no I/O (beyond other imports, although even those should be deferred for third-party modules which don't abide by these rules).

Don't assume that importing your module implies that any of the functionality it provides will actually be used, let alone all of it. It's often the case that A imports B with intent to use some of its functionality, and B imports C because some other functionality provided by B depends upon C, but A never actually makes use of that so it shouldn't be paying a non-trivial price for it.

Also, if a module is only used by a few functions, consider making the import local to the function(s) which actually use it, particularly for modules which have significant dependency trees and which are often otherwise unused.

[martinl]

Replying to zarch:

We are only run the module with the --interface-description and then parsing the xml, I want to avoid to do these operation inside a cycle. Now should be fix.

right, seems to be fixed (1). Thanks.

(1) ​http://wingrass.fsv.cvut.cz/grass70/logs/log-r57623-698/error.log