#2067: PyGRASS grid calls g.region at import time
-----------------------------------------------------------------------------------+
Reporter: huhabla | Owner: grass-dev@…
Type: defect | Status: new
Priority: normal | Milestone: 7.0.0
Component: Python | Version: svn-trunk
Keywords: PyGRASS, Grid, Python, temporal framework, t.remove, t.rast.neighbors | Platform: MSWindows 7
Cpu: Unspecified |
-----------------------------------------------------------------------------------+
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
}}}
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.
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.
> 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.
Replying to [comment:4 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.