[GRASS-dev] PyGRASS Module error handling

[Sören Gebbert]

Hi Vaclav,

2014-06-30 17:43 GMT+02:00 Vaclav Petras <wenzeslaus at gmail.com>:
> Hi,
>
> when writing tests which involve PyGRASS Module class I found that at some
> occasions the error messages are quite confusing.
>
> First when you try to run one module twice, you get AttributeError:
>
>>>> from grass.pygrass.modules import Module
>>>> from subprocess import PIPE
>>>> m = Module('r.info', map='elevation', run_=False, stdout_=PIPE)
>>>> m.run()
>>>> m.outputs['stdout'].value
> ...
>>>> # running for the second time
>>>> m.run()
> Traceback (most recent call last)
>     ...
> AttributeError: 'str' object has no attribute 'fileno'

That should be fixed in r61083. Reason was the wrong handling of
stderr and stdout ,
the PIPE was overwritten when the module finished.

>
> Second, when you confuse m.inputs['map'].value with m.inputs['map'] (which
> is pretty easy error to do) you get "TypeError: deletions are implemented
> differently for unicode":
>
>>>> # this is OK
>>>> m.inputs['map'].value = 5
> Traceback (most recent call last)
>     ...
> TypeError: The Parameter <map>, require: raster, get: <type 'int'> instead
>>>> # this is not OK
>>>> # confusing parameter object and parameter value
>>>> m.inputs['map'] = 'aspect'
> Traceback (most recent call last)
>     ...
> TypeError: deletions are implemented differently for unicode

This is indeed a misleading error report. I do not have a solution right away,
but i would suggest to use always the __call__ function when
specifying an option:

color = Module("r.colors", run_=False)
color(map="test")
color(color="gyr")
color.run()
color(map="test2", color="ryb")
color.run()

> With the former it depends what is actually allowed. Than it is probably
> easy to fix. With the later, no easy solutions come to my mind.
>
> With my tests I was actually asserting m.outputs['stdout'] for true which is
> always true instead of asserting m.inputs['stdout'].value. I don't know how
> to avoid this mistake in general.

We should avoid in general to set the option values directly, instead
the __call__ function should be used
which is much safer.

> Does this make sense to you or do you think that I and PyGRASS users should
> be just be more careful?

The error report must be modified to be more meaningful. I would
suggest to overwrite the assignment function "=" (if this is
possible???) of the Parameter class to catch such errors. The
assignment method will work in case of a Parameter object and raise a
meaningful error in case anything else. Does this sounds reasonable?

Best regards
Soeren

>
> Vaclav