Make autogen/pari Python 3 compatible

[jdemeyer]

CC: @defeo @fchapoton

Component: python3

Author: Frédéric Chapoton

Branch/Commit: 07d262a

Reviewer: Jeroen Demeyer

Issue created by migration from https://trac.sagemath.org/ticket/21874

[fchapoton]

Branch: u/chapoton/21874

[fchapoton]

New commits:

c36156e

21874 py3 compatible autogen/pari (bytes for communicate)

[fchapoton]

Author: Frédéric Chapoton

[fchapoton]

Commit: c36156e

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

6d3d2c7

trac 21874 fixing

[sagetrac-git]

Changed commit from c36156e to 6d3d2c7

[fchapoton]

comment:3

should be good now. Works for python3, not checked for python2.

[jdemeyer]

comment:4

Doctest failure, see patchbot.

[jdemeyer]

comment:5

The "native" format for filenames is bytes. So it seems silly to convert bytes to unicode and then let Python convert it back to bytes. Just drop the .decode().

[fchapoton]

comment:6

elsewhere in the code, one takes the sum (+) of datadir and a string, so one needs the decode()

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

169eb21

trac 21874 correct doctest

[sagetrac-git]

Changed commit from 6d3d2c7 to 169eb21

[jdemeyer]

comment:9

Replying to @fchapoton:

elsewhere in the code, one takes the sum (+) of datadir and a string, so one needs the decode()

That string should then also be bytes.

[defeo]

comment:10

The "native" format for filenames is bytes.

Are you sure about this? In Python 3.5

>>> import os
>>> type(os.listdir('.')[0])
str

[jdemeyer]

comment:11

listdir outputs str when given str and bytes when given bytes:

>>> import os
>>> type(os.listdir(b'.')[0])
<class 'bytes'>

>>> import os
>>> type(os.listdir('.')[0])
<class 'str'>

[jdemeyer]

comment:12

See https://docs.python.org/3/howto/unicode.html#unicode-filenames

Note the sentence "When opening a file for reading or writing, you can usually just provide the Unicode string as the filename, and it will be automatically converted to the right encoding for you". This means that Python does a conversion of encoding, so it makes sense to use bytes for filenames to avoid conversions.

[defeo]

comment:13

Replying to @jdemeyer:

See https://docs.python.org/3/howto/unicode.html#unicode-filenames

Note the sentence "When opening a file for reading or writing, you can usually just provide the Unicode string as the filename, and it will be automatically converted to the right encoding for you". This means that Python does a conversion of encoding, so it makes sense to use bytes for filenames to avoid conversions.

But handling the locale/filesystem encoding seems best left to Python. What if datastring is bytes, string is python's utf-8, and the user's locale is ISO-8859? What is safest among open(datastring.decode() + string) and open(datastring + string.encode())? Probably neither.

I do not even dare imagine what would happen with a WIN-1252-encoded fat filesystem mounted inside a ISO-8859 locale (these things can happen with usb keys...).

[jdemeyer]

comment:14

Replying to @defeo:

But handling the locale/filesystem encoding seems best left to Python. What if datastring is bytes, string is python's utf-8, and the user's locale is ISO-8859? What is safest among open(datastring.decode() + string) and open(datastring + string.encode())? Probably neither.

If string is ASCII, then string.encode() should be pretty safe since most encodings map ASCII to ASCII bytes.

[defeo]

comment:15

Replying to @jdemeyer:

Replying to @defeo:

But handling the locale/filesystem encoding seems best left to Python. What if datastring is bytes, string is python's utf-8, and the user's locale is ISO-8859? What is safest among open(datastring.decode() + string) and open(datastring + string.encode())? Probably neither.

If string is ASCII, then string.encode() should be pretty safe since most encodings map ASCII to ASCII bytes.

Same goes for datastring.decode(), then. I was obviously thinking of the case where they contain non-ASCII characters.

[jdemeyer]

comment:16

Replying to @defeo:

Same goes for datastring.decode(), then.

Not if datastring refers to a user-chosen path. Then we shouldn't make too many assumptions.

[defeo]

comment:17

Replying to @jdemeyer:

Replying to @defeo:

Same goes for datastring.decode(), then.

Not if datastring refers to a user-chosen path. Then we shouldn't make too many assumptions.

Right. I thought it was the other way around. So where are these datadir + string? I see one on line 83 of parser.py, and there string is pari.desc, so I agree with you (assuming that gp correctly handles locale encodings). Any other places?

[fchapoton]

comment:18

I think this is the only place. Could you please handle all this without me ?

[sagetrac-git]

Changed commit from 169eb21 to 07d262a

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

07d262a

trac 21874 using bytes everywhere

[fchapoton]

comment:20

ok, here is another try, following Jeroen's suggestion

[fchapoton]

comment:21

patchbot now gives a green light

[jdemeyer]

Reviewer: Jeroen Demeyer

[jdemeyer]

comment:22

Good for me then.

[vbraun]

Changed branch from u/chapoton/21874 to 07d262a