pygrass: backport #1407 Module fix (#1606)

lib/python/docs/src/pygrass_modules.rst

@@ -168,32 +168,16 @@ User or developer can check which parameters have been set, with: ::
         print "Aspect is not computed"
 
 
-After we set the parameters and run the module, the execution of the module
-instantiate a popen attribute to the class. The `Popen`_ class allow user
-to kill/wait/ the process. ::
+After we set the parameters, we can run the module in the background with
+`finish_=False`. Then call `wait()` and retrieve the returncode. ::
 
     >>> slope_aspect = Module('r.slope.aspect')
     >>> slope_aspect(elevation='elevation', slope='slp', aspect='asp', overwrite=True, finish_=False)
-    >>> slope_aspect.popen.wait() # *.kill(), *.terminate()
+    >>> slope_aspect.wait()
+    Module('r.slope.aspect')
+    >>> slope_aspect.returncode
     0
-    >>> out, err = slope_aspect.popen.communicate()
-    >>> print err #doctest: +NORMALIZE_WHITESPACE
-     100%
-    Aspect raster map <asp> complete
-    Slope raster map <slp> complete
-    <BLANKLINE>
 
-On the above example we use a new parameter `finish_`, if is set to True, the
-run method, automatically store the stdout and stderr to stdout and stderr
-attributes of the class: ::
-
-    >>> slope_aspect = Module('r.slope.aspect')
-    >>> slope_aspect(elevation='elevation', slope='slp', aspect='asp', overwrite=True, finish_=True)
-    >>> print slope_aspect.stderr #doctest: +NORMALIZE_WHITESPACE
-     100%
-    Aspect raster map <asp> complete
-    Slope raster map <slp> complete
-    <BLANKLINE>
 
 Another example of use: ::
 
@@ -202,9 +186,7 @@ Another example of use: ::
     >>> from grass.script.utils import parse_key_val
     >>> parse_key_val(info.outputs.stdout)
     {'max': '156.3299', 'min': '55.57879'}
-    >>> info = Module("r.info", map="elevation", flags="r", finish_=False)
-    >>> category = Module("r.category", map="elevation",
-    ...                   stdin_=info.popen.stdout, finish_=True)
+
 
 Launching GRASS GIS modules in parallel
 ---------------------------------------

lib/python/gunittest/case.py

@@ -1093,9 +1093,9 @@ class TestCase(unittest.TestCase):
                 errors += "\nSee available vector maps:\n"
                 errors += call_module('g.list', type='vector')
             # TODO: message format, parameters
-            raise CalledModuleError(module.popen.returncode, module.name,
-                                    module.get_python(),
-                                    errors=errors)
+            raise CalledModuleError(
+                module.returncode, module.name, module.get_python(), errors=errors
+            )
         # TODO: use this also in assert and apply when appropriate
         if expecting_stdout and not module.outputs.stdout.strip():
 
@@ -1153,14 +1153,15 @@ class TestCase(unittest.TestCase):
             print(text_to_string(module.outputs.stderr))
             # TODO: message format
             # TODO: stderr?
-            stdmsg = ('Running <{m.name}> module ended'
-                      ' with non-zero return code ({m.popen.returncode})\n'
-                      'Called: {code}\n'
-                      'See the following errors:\n'
-                      '{errors}'.format(
-                          m=module, code=module.get_python(),
-                          errors=module.outputs.stderr
-                      ))
+            stdmsg = (
+                "Running <{m.name}> module ended"
+                " with non-zero return code ({m.returncode})\n"
+                "Called: {code}\n"
+                "See the following errors:\n"
+                "{errors}".format(
+                    m=module, code=module.get_python(), errors=module.outputs.stderr
+                )
+            )
             self.fail(self._formatMessage(msg, stdmsg))
         print(text_to_string(module.outputs.stdout))
         print(text_to_string(module.outputs.stderr))

lib/python/gunittest/gmodules.py

@@ -26,14 +26,14 @@ class SimpleModule(Module):
     ...                        overwrite=True)
     >>> mapcalc.run()
     Module('r.mapcalc')
-    >>> mapcalc.popen.returncode
+    >>> mapcalc.returncode
     0
 
     >>> colors = SimpleModule('r.colors',
     ...                       map='test_a', rules='-', stdin_='1 red')
     >>> colors.run()
     Module('r.colors')
-    >>> colors.popen.returncode
+    >>> colors.returncode
     0
     >>> str(colors.inputs.stdin)
     '1 red'

lib/python/pygrass/modules/interface/module.py

@@ -53,7 +53,7 @@ class ParallelModuleQueue(object):
     Check with a queue size of 3 and 5 processes
 
     >>> import copy
-    >>> from grass.pygrass.modules import Module, ParallelModuleQueue
+    >>> from grass.pygrass.modules import Module, MultiModule, ParallelModuleQueue
     >>> mapcalc_list = []
 
     Setting run_ to False is important, otherwise a parallel processing is not possible
@@ -72,7 +72,7 @@ class ParallelModuleQueue(object):
     >>> queue.get_max_num_procs()
     3
     >>> for mapcalc in mapcalc_list:
-    ...     print(mapcalc.popen.returncode)
+    ...     print(mapcalc.returncode)
     0
     0
     0
@@ -95,7 +95,7 @@ class ParallelModuleQueue(object):
     >>> queue.get_max_num_procs()
     8
     >>> for mapcalc in mapcalc_list:
-    ...     print(mapcalc.popen.returncode)
+    ...     print(mapcalc.returncode)
     0
     0
     0
@@ -122,7 +122,7 @@ class ParallelModuleQueue(object):
     >>> queue.get_max_num_procs()
     3
     >>> for proc in proc_list:
-    ...     print(proc.popen.returncode)
+    ...     print(proc.returncode)
     0
     0
     0
@@ -165,7 +165,7 @@ class ParallelModuleQueue(object):
     >>> queue.get_max_num_procs()
     8
     >>> for mapcalc in mapcalc_list:
-    ...     print(mapcalc.popen.returncode)
+    ...     print(mapcalc.returncode)
     0
     0
     0
@@ -206,7 +206,7 @@ class ParallelModuleQueue(object):
     >>> queue.get_max_num_procs()
     3
     >>> for mapcalc in mapcalc_list:
-    ...     print(mapcalc.popen.returncode)
+    ...     print(mapcalc.returncode)
     0
     0
     0
@@ -366,7 +366,7 @@ class Module(object):
     ...                  overwrite=True, run_=False)
     >>> mapcalc.run()
     Module('r.mapcalc')
-    >>> mapcalc.popen.returncode
+    >>> mapcalc.returncode
     0
 
     >>> mapcalc = Module("r.mapcalc", expression="test_a = 1",
@@ -374,12 +374,12 @@ class Module(object):
     >>> mapcalc.run()
     Module('r.mapcalc')
     >>> p = mapcalc.wait()
-    >>> p.popen.returncode
+    >>> p.returncode
     0
     >>> mapcalc.run()
     Module('r.mapcalc')
     >>> p = mapcalc.wait()
-    >>> p.popen.returncode
+    >>> p.returncode
     0
 
     >>> colors = Module("r.colors", map="test_a", rules="-",
@@ -388,7 +388,7 @@ class Module(object):
     >>> colors.run()
     Module('r.colors')
     >>> p = mapcalc.wait()
-    >>> p.popen.returncode
+    >>> p.returncode
     0
     >>> colors.inputs["stdin"].value
     '1 red'
@@ -399,43 +399,47 @@ class Module(object):
 
     >>> colors = Module("r.colors", map="test_a", rules="-",
     ...                 run_=False, finish_=False, stdin_=PIPE)
+    >>> colors.inputs["stdin"].value = "1 red"
     >>> colors.run()
     Module('r.colors')
-    >>> stdout, stderr = colors.popen.communicate(input=b"1 red")
-    >>> colors.popen.returncode
+    >>> colors.wait()
+    Module('r.colors')
+    >>> colors.returncode
     0
-    >>> stdout
-    >>> stderr
 
     >>> colors = Module("r.colors", map="test_a", rules="-",
     ...                 run_=False, finish_=False,
     ...                 stdin_=PIPE, stderr_=PIPE)
+    >>> colors.inputs["stdin"].value = "1 red"
     >>> colors.run()
     Module('r.colors')
-    >>> stdout, stderr = colors.popen.communicate(input=b"1 red")
-    >>> colors.popen.returncode
+    >>> colors.wait()
+    Module('r.colors')
+    >>> colors.outputs["stderr"].value.strip()
+    "Color table for raster map <test_a> set to 'rules'"
+
+    >>> colors.returncode
     0
-    >>> stdout
-    >>> stderr.strip()
-    b"Color table for raster map <test_a> set to 'rules'"
 
     Run a second time
 
+    >>> colors.inputs["stdin"].value = "1 red"
     >>> colors.run()
     Module('r.colors')
-    >>> stdout, stderr = colors.popen.communicate(input=b"1 blue")
-    >>> colors.popen.returncode
+    >>> colors.wait()
+    Module('r.colors')
+    >>> colors.outputs["stderr"].value.strip()
+    "Color table for raster map <test_a> set to 'rules'"
+
+    >>> colors.returncode
     0
-    >>> stdout
-    >>> stderr.strip()
-    b"Color table for raster map <test_a> set to 'rules'"
 
     Multiple run test
 
     >>> colors = Module("r.colors", map="test_a",
     ...                                            color="ryb", run_=False)
     >>> colors.get_bash()
-    'r.colors map=test_a color=ryb'
+    'r.colors map=test_a color=ryb offset=0.0 scale=1.0'
     >>> colors.run()
     Module('r.colors')
     >>> colors(color="gyr")
@@ -575,18 +579,24 @@ class Module(object):
         self.stdin = None
         self.stdout_ = None
         self.stderr_ = None
-        diz = {'name': 'stdin', 'required': False,
-               'multiple': False, 'type': 'all',
-               'value': None}
-        self.inputs['stdin'] = Parameter(diz=diz)
-        diz['name'] = 'stdout'
-        self.outputs['stdout'] = Parameter(diz=diz)
-        diz['name'] = 'stderr'
-        self.outputs['stderr'] = Parameter(diz=diz)
-        self.popen = None
+        diz = {
+            "name": "stdin",
+            "required": False,
+            "multiple": False,
+            "type": "all",
+            "value": None,
+        }
+        self.inputs["stdin"] = Parameter(diz=diz)
+        diz["name"] = "stdout"
+        self.outputs["stdout"] = Parameter(diz=diz)
+        diz["name"] = "stderr"
+        self.outputs["stderr"] = Parameter(diz=diz)
+        self._popen = None
         self.time = None
-        self.start_time = None            # This variable will be set in the run() function
-        self._finished = False            # This variable is set True if wait() was successfully called
+        self.start_time = None  # This variable will be set in the run() function
+        # This variable is set True if wait() was successfully called
+        self._finished = False
+        self.returncode = None
 
         if args or kargs:
             self.__call__(*args, **kargs)
@@ -761,11 +771,13 @@ class Module(object):
 
         cmd = self.make_cmd()
         self.start_time = time.time()
-        self.popen = Popen(cmd,
-                           stdin=self.stdin_,
-                           stdout=self.stdout_,
-                           stderr=self.stderr_,
-                           env=self.env_)
+        self._popen = Popen(
+            cmd,
+            stdin=self.stdin_,
+            stdout=self.stdout_,
+            stderr=self.stderr_,
+            env=self.env_,
+        )
 
         if self.finish_ is True:
             self.wait()
@@ -781,17 +793,21 @@ class Module(object):
         if self._finished is False:
             if self.stdin:
                 self.stdin = encode(self.stdin)
-            stdout, stderr = self.popen.communicate(input=self.stdin)
-            self.outputs['stdout'].value = decode(stdout) if stdout else ''
-            self.outputs['stderr'].value = decode(stderr) if stderr else ''
+            stdout, stderr = self._popen.communicate(input=self.stdin)
+            self.outputs["stdout"].value = decode(stdout) if stdout else ""
+            self.outputs["stderr"].value = decode(stderr) if stderr else ""
             self.time = time.time() - self.start_time
-
+            self.returncode = self._popen.returncode
             self._finished = True
 
-            if self.popen.poll():
-                raise CalledModuleError(returncode=self.popen.returncode,
-                                        code=self.get_bash(),
-                                        module=self.name, errors=stderr)
+            if self._popen.poll():
+                raise CalledModuleError(
+                    returncode=self._popen.returncode,
+                    code=self.get_bash(),
+                    module=self.name,
+                    errors=stderr,
+                )
+        self._popen = None
 
         return self
 
@@ -836,38 +852,38 @@ class MultiModule(object):
     >>> mm = MultiModule(module_list=[region_1, region_2])
     >>> mm.run()
     >>> m_list = mm.get_modules()
-    >>> m_list[0].popen.returncode
+    >>> m_list[0].returncode
     0
-    >>> m_list[1].popen.returncode
+    >>> m_list[1].returncode
     0
 
     Asynchronous module run, setting finish = False
 
-    >>> region_1 = Module("g.region", run_=False)  # doctest: +SKIP
-    >>> region_1.flags.p = True  # doctest: +SKIP
-    >>> region_2 = copy.deepcopy(region_1)  # doctest: +SKIP
-    >>> region_2.flags.p = True  # doctest: +SKIP
-    >>> region_3 = copy.deepcopy(region_1)  # doctest: +SKIP
-    >>> region_3.flags.p = True  # doctest: +SKIP
-    >>> region_4 = copy.deepcopy(region_1)  # doctest: +SKIP
-    >>> region_4.flags.p = True  # doctest: +SKIP
-    >>> region_5 = copy.deepcopy(region_1)  # doctest: +SKIP
-    >>> region_5.flags.p = True  # doctest: +SKIP
+    >>> region_1 = Module("g.region", run_=False)
+    >>> region_1.flags.p = True
+    >>> region_2 = copy.deepcopy(region_1)
+    >>> region_2.flags.p = True
+    >>> region_3 = copy.deepcopy(region_1)
+    >>> region_3.flags.p = True
+    >>> region_4 = copy.deepcopy(region_1)
+    >>> region_4.flags.p = True
+    >>> region_5 = copy.deepcopy(region_1)
+    >>> region_5.flags.p = True
     >>> mm = MultiModule(module_list=[region_1, region_2, region_3, region_4, region_5],
-    ...                  sync=False)  # doctest: +SKIP
-    >>> t = mm.run()  # doctest: +SKIP
-    >>> isinstance(t, Process)  # doctest: +SKIP
+    ...                  sync=False)
+    >>> t = mm.run()
+    >>> isinstance(t, Process)
     True
-    >>> m_list = mm.wait()  # doctest: +SKIP
-    >>> m_list[0].popen.returncode  # doctest: +SKIP
+    >>> m_list = mm.wait()
+    >>> m_list[0].returncode
     0
-    >>> m_list[1].popen.returncode  # doctest: +SKIP
+    >>> m_list[1].returncode
     0
-    >>> m_list[2].popen.returncode  # doctest: +SKIP
+    >>> m_list[2].returncode
     0
-    >>> m_list[3].popen.returncode  # doctest: +SKIP
+    >>> m_list[3].returncode
     0
-    >>> m_list[4].popen.returncode  # doctest: +SKIP
+    >>> m_list[4].returncode
     0
 
     Asynchronous module run, setting finish = False and using temporary region
@@ -880,15 +896,15 @@ class MultiModule(object):
     >>> isinstance(t, Process)
     True
     >>> m_list = mm.wait()
-    >>> m_list[0].popen.returncode
+    >>> m_list[0].returncode
     0
-    >>> m_list[1].popen.returncode
+    >>> m_list[1].returncode
     0
-    >>> m_list[2].popen.returncode
+    >>> m_list[2].returncode
     0
-    >>> m_list[3].popen.returncode
+    >>> m_list[3].returncode
     0
-    >>> m_list[4].popen.returncode
+    >>> m_list[4].returncode
     0
 
     """

lib/python/pygrass/modules/interface/typedict.py

@@ -55,6 +55,18 @@ class TypeDict(OrderedDict):
             obj[k] = deepcopy(v)
         return obj
 
+    def __reduce__(self):
+        inst_dict = vars(self).copy()
+        for k in vars(TypeDict(self._type)):
+            inst_dict.pop(k, None)
+        return (
+            self.__class__,
+            (self._type,),
+            inst_dict or None,
+            None,
+            iter(self.items()),
+        )
+
     def used(self):
         key_dict = {}
         for key in self:

lib/python/temporal/temporal_vector_algebra.py

@@ -416,11 +416,12 @@ class TemporalVectorAlgebraParser(TemporalAlgebraParser):
                             self.msgr.message("Run command:\n" + cmd.get_bash())
                             cmd.run()
 
-                            if cmd.popen.returncode != 0:
-                                self.msgr.fatal(_("Error starting %s : \n%s")
-                                                  %(cmd.get_bash(),
-                                                  cmd.popen.stderr))
-                            mapname = cmd.outputs['output'].value
+                            if cmd.returncode != 0:
+                                self.msgr.fatal(
+                                    _("Error starting %s : \n%s")
+                                    % (cmd.get_bash(), cmd.outputs.stderr)
+                                )
+                            mapname = cmd.outputs["output"].value
                             if mapname.find("@") >= 0:
                                 map_test = map_i.get_new_instance(mapname)
                             else:
@@ -490,9 +491,13 @@ class TemporalVectorAlgebraParser(TemporalAlgebraParser):
                                 map_i.update_all(dbif=dbif)
                             elif map_i.is_in_db(dbif=dbif) and self.overwrite == False:
                                 # Raise error if map exists and no overwrite flag is given.
-                                self.msgr.fatal(_("Error vector map %s exist in temporal database. "
-                                                  "Use overwrite flag.  : \n%s") \
-                                                  %(map_i.get_map_id(), cmd.popen.stderr))
+                                self.msgr.fatal(
+                                    _(
+                                        "Error vector map %s exist in temporal database. "
+                                        "Use overwrite flag.  : \n%s"
+                                    )
+                                    % (map_i.get_map_id(), cmd.outputs.stderr)
+                                )
                             else:
                                 # Insert map into temporal database.
                                 map_i.insert(dbif=dbif)

temporal/t.rast.accumulate/t.rast.accumulate.py

@@ -469,7 +469,7 @@ def main():
             print(accmod)
             accmod.run()
 
-            if accmod.popen.returncode != 0:
+            if accmod.returncode != 0:
                 dbif.close()
                 grass.fatal(_("Error running r.series.accumulate"))
 

temporal/t.rast.neighbors/t.rast.neighbors.py

@@ -194,8 +194,11 @@ def main():
     # Check return status of all finished modules
     error = 0
     for proc in proc_list:
-        if proc.popen.returncode != 0:
-            grass.error(_("Error running module: %\n    stderr: %s") %(proc.get_bash(), proc.outputs.stderr))
+        if proc.returncode != 0:
+            grass.error(
+                _("Error running module: %\n    stderr: %s")
+                % (proc.get_bash(), proc.outputs.stderr)
+            )
             error += 1
 
     if error > 0: