[GRASS-dev] [GRASS GIS] #2708: Run GRASS with Python3

#2708: Run GRASS with Python3
-------------------------+-------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Keywords: | CPU: Unspecified
Platform: Unspecified |
-------------------------+-------------------------
Dear all,

I've started playing with GRASS and Python3. I did some testing on
Python2.6, Python2.7 and Python3.4.

So far I'm able to start a GRASS session and execute something trivial
command, like:

{{{
python -c "from grass.script import read_command;
print(read_command('g.region', flags='p'))"
}}}

GRASS under python2 with these changes seems to work fine on my system.

I've splitted the changes in three main patchs:

1) lib/init/grass.py => init_grass.diff

2) lib/python/gunittest/* => gunittest.diff

3) lib/python/script/* => script.diff

My question is how can we share these changes? should we create a separate
branch? may I just put these changes on trunk and break it :-D, or should
trunk still remain quite stable with not experimental code?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2708&gt;
GRASS GIS <http://grass.osgeo.org>

#2708: Run GRASS with Python3
--------------------------+-------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------
Changes (by zarch):

* Attachment "init_grass.diff" added.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2708&gt;
GRASS GIS <http://grass.osgeo.org>

#2708: Run GRASS with Python3
--------------------------+-------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------
Changes (by zarch):

* Attachment "gunittest.diff" added.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2708&gt;
GRASS GIS <http://grass.osgeo.org>

#2708: Run GRASS with Python3
--------------------------+-------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------
Changes (by zarch):

* Attachment "script.diff" added.

--
Ticket URL: <http://trac.osgeo.org/grass/ticket/2708&gt;
GRASS GIS <http://grass.osgeo.org>

#2708: Run GRASS with Python3
--------------------------+-------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by glynn):

Replying to [ticket:2708 zarch]:

> 3) lib/python/script/* => script.diff

These are wrong. You should be converting str to bytes, not the other way
around. A child process' argv, environment, stdin/stdout/stderr are all
byte sequences. Converting everything to unicode so that Python can
convert it back to bytes under the hood is adding needless failure modes.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2708#comment:1&gt;
GRASS GIS <http://grass.osgeo.org>

#2708: Run GRASS with Python3
--------------------------+-------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by zarch):

Replying to [comment:1 glynn]:
> Replying to [ticket:2708 zarch]:
>
> > 3) lib/python/script/* => script.diff
>
> These are wrong. You should be converting str to bytes, not the other
way around.
> A child process' argv, environment, stdin/stdout/stderr are all byte
sequences.
> Converting everything to unicode so that Python can convert it back to
bytes under
> the hood is adding needless failure modes.

I agree with you, actually this was my first attempt, but then I have to
face that the string formatting is available for bytes, only on python
3.5, that it is not stable yet. therefore when I call
make_command/run_command I got:

{{{
In [1]: from grass.script import core as gcore

In [2]: gcore.make_command('g.region', raster='elevation', flags='p')
Out[2]: ['g.region', '-p', "raster=b'elevation'"]

In [3]: gcore.run_command('g.region', raster='elevation', flags='p')
WARNING: Illegal filename <b'elevation'>. Character <'> not allowed.
ERROR: Raster map <b'elevation'> not found
---------------------------------------------------------------------------
CalledModuleError Traceback (most recent call
last)
<ipython-input-5-c7b3927bef11> in <module>()
----> 1 gcore.run_command('g.region', raster='elevation', flags='p')

/home/pietro/docdat/src/gis/grass71/dist.x86_64-unknown-linux-
gnu/etc/python/grass/script/core.py in run_command(*args, **kwargs)
     403 ps = start_command(*args, **kwargs)
     404 returncode = ps.wait()
--> 405 return handle_errors(returncode, returncode, args, kwargs)
     406
     407

/home/pietro/docdat/src/gis/grass71/dist.x86_64-unknown-linux-
gnu/etc/python/grass/script/core.py in handle_errors(returncode, result,
args, kwargs)
     321 args = make_command(*args, **kwargs)
     322 raise CalledModuleError(module=None, code=repr(args),
--> 323 returncode=returncode)
     324
     325 def start_command(prog, flags="", overwrite=False, quiet=False,

CalledModuleError: Module run None ['g.region', '-p',
"raster=b'elevation'"] ended with error
Process ended with non-zero return code 1. See errors in the (error)
output.

In [4]: "%s=%s" % (b'raster', b'elevation')
Out[4]: "b'raster'=b'elevation'"

In [5]: "%s=%s" % (b'raster'.decode(), b'elevation'.decode())
Out[5]: 'raster=elevation'
}}}

Do you have an idea on how we could/should fix this?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2708#comment:2&gt;
GRASS GIS <http://grass.osgeo.org>

#2708: Run GRASS with Python3
--------------------------+-------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by glynn):

Replying to [comment:2 zarch]:

>
{{{
In [5]: "%s=%s" % (b'raster'.decode(), b'elevation'.decode())
Out[5]: 'raster=elevation'
}}}
>
> Do you have an idea on how we could/should fix this?

Just avoid using string formatting for such trivial cases, e.g.:

{{{
args.append(opt.encode('ascii') + b'=' + _make_val(val)
}}}

To be honest, converting grass.script to Python 3 isn't going to be much
fun, as a scripting library fundamentally revolves around dealing with
byte strings (command-line arguments, environment variables,
stdin/stdout), while Python 3 tries to pretend that byte strings are some
kind of low-level implementation detail in a world where everything is
Unicode.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2708#comment:3&gt;
GRASS GIS <http://grass.osgeo.org>

#2708: Run GRASS with Python3
--------------------------+-------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by zarch):

Replying to [comment:3 glynn]:
> {{{
> args.append(opt.encode('ascii') + b'=' + _make_val(val)
> }}}
>
> To be honest, converting grass.script to Python 3 isn't going to be much
fun,
> as a scripting library fundamentally revolves around dealing with byte
> strings (command-line arguments, environment variables, stdin/stdout),
> while Python 3 tries to pretend that byte strings are some kind of low-
level
> implementation detail in a world where everything is Unicode.

ok, I've followed your approach, now the make_command it is working only
with bytes and return a list of bytes.
Do you think that I could commit these changes in trunk? Other things that
should be change before?

I've also started fixing several GRASS scripts to be parsed also with
python3.

However I don't know how to fix the ctypes binding of GRASS, at the moment
I get:

{{{
In [1]: from grass.lib import gis
---------------------------------------------------------------------------
ImportError Traceback (most recent call
last)
<ipython-input-1-9c20dc5e4f67> in <module>()
----> 1 from grass.lib import gis

/home/pietro/docdat/src/gis/grass71/dist.x86_64-unknown-linux-
gnu/etc/python/grass/lib/gis.py in <module>()
      13 _libdirs =
      14
---> 15 from ctypes_preamble import *
      16 from ctypes_preamble import _variadic_function
      17 from ctypes_loader import *

ImportError: No module named 'ctypes_preamble'
}}}

the problem here is that the gis.py file should start with:

{{{
from __future__ import absolute_import

...

from .ctypes_preamble import *
from .ctypes_preamble import _variadic_function
from .ctypes_loader import *

}}}

But these files are generated with make and I don't understand where
should I change the code. Any hint?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2708#comment:4&gt;
GRASS GIS <http://grass.osgeo.org>

#2708: Run GRASS with Python3
--------------------------+-------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------
Changes (by zarch):

* Attachment "script.2.diff" added.

reviewed version, use bytes instead of string (unicode)

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2708&gt;
GRASS GIS <http://grass.osgeo.org>

#2708: Run GRASS with Python3
--------------------------+-------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by glynn):

Replying to [comment:4 zarch]:
> Do you think that I could commit these changes in trunk?

What's the oldest Python 2.x version they will work with?

Support for 2.7 is essential. We've been trying to maintain support for
2.6, but that may have to change in order to support 3.x.

> However I don't know how to fix the ctypes binding of GRASS, at the
moment I get:

> But these files are generated with make and I don't understand where
should I change the code. Any hint?

The lib/python/ctypes/fix.sed script inserts those imports.

If the `__future__` import needs to be at the very start of the file, add
the command
{{{
1i \
from __future__ import absolute_import
}}}
to the script.

If it just needs to be before any imports, add it to the start of the
existing imports in the script.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2708#comment:5&gt;
GRASS GIS <http://grass.osgeo.org>

#2708: Run GRASS with Python3
--------------------------+-------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by zarch):

Replying to [comment:5 glynn]:
> Replying to [comment:4 zarch]:
> > Do you think that I could commit these changes in trunk?
>
> What's the oldest Python 2.x version they will work with?

I did some test with:
- Python 2.6.9
- Python 2.7.10
- Python 3.4.3

> Support for 2.7 is essential. We've been trying to maintain support for
2.6,
> but that may have to change in order to support 3.x.

I thjink should be possible to write code that it is compatible with
2.6/2.7 and 3.4.
I will test for few more days and then I will commit in trunk if there are
no objections

> > But these files are generated with make and I don't understand where
should I change the code. Any hint?
>
> The lib/python/ctypes/fix.sed script inserts those imports.

Thank you it works!

Now I get:

{{{
In [1]: from grass.lib import gis
---------------------------------------------------------------------------
TypeError Traceback (most recent call
last)
<ipython-input-1-9c20dc5e4f67> in <module>()
----> 1 from grass.lib import gis

/home/pietro/docdat/src/gis/grass71/dist.x86_64-unknown-linux-
gnu/etc/python/grass/lib/gis.py in <module>()
     800 ]
     801 struct_anon_11._fields_ = [
--> 802 ('__val', c_ulong * (1024 / (8 * sizeof(c_ulong)))),
     803 ]
     804

TypeError: can't multiply sequence by non-int of type 'float'
}}}

Testing with the python debugger:

{{{
ipdb> (1024 / (8 * sizeof(c_ulong)))
16.0
ipdb> c_ulong * (1024 / (8 * sizeof(c_ulong)))
*** TypeError: can't multiply sequence by non-int of type 'float'
}}}

Do you have an idea on how we can fix this?

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2708#comment:6&gt;
GRASS GIS <http://grass.osgeo.org>

#2708: Run GRASS with Python3
--------------------------+-------------------------
  Reporter: zarch | Owner: grass-dev@…
      Type: defect | Status: new
  Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
       CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by glynn):

Replying to [comment:6 zarch]:

> Testing with the python debugger:
>
{{{
ipdb> (1024 / (8 * sizeof(c_ulong)))
16.0
ipdb> c_ulong * (1024 / (8 * sizeof(c_ulong)))
*** TypeError: can't multiply sequence by non-int of type 'float'
}}}
>
> Do you have an idea on how we can fix this?

One option is to make ctypesgen use truncating division:

lib/python/ctypes/ctypesgencore/parser/cgrammar.py:282
{{{
- '/': ("division", (lambda x,y: x/y), "(%s / %s)"),
+ '/': ("division", (lambda x,y: x/y), "(%s // %s)"),
}}}

However, this would break macros which perform floating-point division.

Another option would be to explicitly convert array sizes to integers, but
that still doesn't handle the situation where a macro is expecting "/" to
match C's division semantics (truncating division for integers, non-
truncating division for floating-point values).

Ultimately, ctypesgen just translates macros directly to Python, so
whichever division operator is used would be wrong for one case or the
other. We could change it to use e.g. c_division(a,b) and define that
function in ctypes_preamble.py. But that seems like overkill given that
(on Linux) there are two occurrences of the division operator in the
generated files.

One of them is for the definition of sigset_t, which is required for
jmp_buf which in turn is required for G_fatal_longjmp(), and I'm not sure
it's even possible to make use of that from Python. The other is for a
macro named FUDGE() in ogsf.h, which I suspect is probably not
particularly useful (and that one happens to be a floating-point
division).

An alternative option is to just guard the <setjmp.h> include and the
G_fatal_longjmp() declaration in defs/gis.h with `#ifndef CTYPESGEN`, and
do likewise for the FUDGE() macro in ogsf.h.

However, that ignores the possibility that other platforms may have macros
involving division in their system headers, or that future changes to
GRASS may pull in additional headers with such macros.

Yet another option is a combination of the other two: prevent ctypesgen
from ever seeing a macro involving division, and just remove the division
case from mult_ops_dict so that if it does encounter one it raises an
exception. That may require ongoing maintenance but avoids the situation
where we end up silently generating broken conversions of macros.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2708#comment:7&gt;
GRASS GIS <http://grass.osgeo.org>

Hi,

is this the right ticket to look into the porting of grass to work with python3 ?
what’s the state of port, is there any temporary fork of the project or should I apply the several patches on this ticket ?

At the moment my build fails on most of core modules my default python is V3.
Is there a way to specify in the configure which python has to be used for to build grass ?

Thanks!

Massimo.

On Jul 24, 2015, at 9:57 AM, GRASS GIS <trac@osgeo.org> wrote:

#2708: Run GRASS with Python3
--------------------------+-------------------------
Reporter: zarch | Owner: grass-dev@…
     Type: defect | Status: new
Priority: normal | Milestone: 7.0.1
Component: Default | Version: unspecified
Resolution: | Keywords:
      CPU: Unspecified | Platform: Unspecified
--------------------------+-------------------------

Comment (by glynn):

Replying to [comment:6 zarch]:

Testing with the python debugger:

{{{
ipdb> (1024 / (8 * sizeof(c_ulong)))
16.0
ipdb> c_ulong * (1024 / (8 * sizeof(c_ulong)))
*** TypeError: can't multiply sequence by non-int of type 'float'
}}}

Do you have an idea on how we can fix this?

One option is to make ctypesgen use truncating division:

lib/python/ctypes/ctypesgencore/parser/cgrammar.py:282
{{{
- '/': ("division", (lambda x,y: x/y), "(%s / %s)"),
+ '/': ("division", (lambda x,y: x/y), "(%s // %s)"),
}}}

However, this would break macros which perform floating-point division.

Another option would be to explicitly convert array sizes to integers, but
that still doesn't handle the situation where a macro is expecting "/" to
match C's division semantics (truncating division for integers, non-
truncating division for floating-point values).

Ultimately, ctypesgen just translates macros directly to Python, so
whichever division operator is used would be wrong for one case or the
other. We could change it to use e.g. c_division(a,b) and define that
function in ctypes_preamble.py. But that seems like overkill given that
(on Linux) there are two occurrences of the division operator in the
generated files.

One of them is for the definition of sigset_t, which is required for
jmp_buf which in turn is required for G_fatal_longjmp(), and I'm not sure
it's even possible to make use of that from Python. The other is for a
macro named FUDGE() in ogsf.h, which I suspect is probably not
particularly useful (and that one happens to be a floating-point
division).

An alternative option is to just guard the <setjmp.h> include and the
G_fatal_longjmp() declaration in defs/gis.h with `#ifndef CTYPESGEN`, and
do likewise for the FUDGE() macro in ogsf.h.

However, that ignores the possibility that other platforms may have macros
involving division in their system headers, or that future changes to
GRASS may pull in additional headers with such macros.

Yet another option is a combination of the other two: prevent ctypesgen
from ever seeing a macro involving division, and just remove the division
case from mult_ops_dict so that if it does encounter one it raises an
exception. That may require ongoing maintenance but avoids the situation
where we end up silently generating broken conversions of macros.

--
Ticket URL: <https://trac.osgeo.org/grass/ticket/2708#comment:7&gt;
GRASS GIS <http://grass.osgeo.org>

_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
http://lists.osgeo.org/mailman/listinfo/grass-dev

On Wed, Sep 23, 2015 at 4:28 AM, massimo di stefano
<massimodisasha@gmail.com> wrote:

Hi,

is this the right ticket to look into the porting of grass to work with python3 ?

I guess so (but this is the ML, not trac)

what’s the state of port, is there any temporary fork of the project or should I apply the several patches on this ticket ?

Please do.

At the moment my build fails on most of core modules my default python is V3.
Is there a way to specify in the configure which python has to be used for to build grass ?

AFAIK: when running configure:

--with-python=/usr/bin/python3.4-config

best
Markus

Hi Massimo,

On Wed, Sep 23, 2015 at 4:28 AM, massimo di stefano
<massimodisasha@gmail.com> wrote:

what’s the state of port, is there any temporary fork of the project or should I apply the several patches on this ticket ?

So far all the grass/script library it seems to work (and it is
already in trunk), therefore you don't need to apply any patch.
The ctypes are still not working, but I have not the deep
understanding of this code, therefore it is difficult for me to port
them. Therefore all the libraries and modules that are refering to the
ctypes are not working on python3 (pygrass, temporal, etc).

Many python modules are not working out of the box for python 3, In
August I've changed them and I'm using them for testing since for most
of them the testsuite is missing.
May be it is time to sync my local changes and trunk.

The GUI will not start on python3, the wxpython became Phoenix:

- http://wiki.wxpython.org/ProjectPhoenix
- http://wxpython.org/Phoenix/docs/html/main.html

Therefore if you want the GUI I don't think that python3 could be an
option (in short time at least).

At the moment my build fails on most of core modules my default python is V3.

As I said, I've started fixing most of them... if interested I could
sync my local changes to trunk next week.
However you can compile with python2 and execute with python3.

Is there a way to specify in the configure which python has to be used for to build grass?

I personally use a virtualenv.
If I remeber correctly Glynn suggest to create a link in the grass
source root of your python binary.

IMHO use GRASS with python3, only for testing and contributing to the
port... not for your daily work.

All the best.

Pietro