search

translate / virtaal

Easy-to-use and powerful offline translation tool
https://virtaal.translatehouse.org
GNU General Public License v2.0
298 stars 57 forks source link

Misc Python 3 porting fixups #3308

Closed llimeht closed 4 years ago

llimeht commented 4 years ago

A few additional things to fix found either when running the program or running py3compile over all files as part of updating the Debian packaging.

llimeht commented 4 years ago

@friedelwolff I guess merge conflicts here were entirely predictable. Do you want me to rebase or will you just cherry-pick the bits that are useful?

friedelwolff commented 4 years ago

Sorry about that. I had a lot of changes queued, and we didn't see each other's work. I'll try to go through this, but my time is now less from today on. I'll try to see if you had some better solutions.

Thank you for the contribution.

llimeht commented 4 years ago

No worries @friedelwolff! I think we had either the same solution or you had a better solution to several of them. I'll try to rebase the branch to make it easy to see what else I had found.

llimeht commented 4 years ago

Rebased on top of the py3 branch with duplicated work dropped as appropriate.

I've also rebased the tmp/py3-travis branch on top of this PR so that the tests in #3309 actually pass.

rffontenelle commented 4 years ago

@llimeht by running grep -R ' u"' in the root directory of your temporary branch, I notice occurrence of strings being set as Unicode. AFAIK is needed for Python 2 only. Can you please check if these u can be removed too?

rffontenelle commented 4 years ago

Same applies to grep -R " u'"

rffontenelle commented 4 years ago

2to3 seems to be very handy on the migration.

llimeht commented 4 years ago

There are other u"…" constructs in the code base that are cruft and can be fixed some time in the future. There is no rush. They are a no-op from Python 3.3 onwards (see PEP 414).

There is only one ur"…" in the code base; ur"…" is a syntax error and so needs fixing not merely decrufting.

$ python2.7
>>> punc_symbols = ur'''.,?!:;'"“”‘’—)'''
>>> 
$ python3.7
>>> punc_symbols = ur'''.,?!:;'"“”‘’—)'''
  File "<stdin>", line 1
    punc_symbols = ur'''.,?!:;'"“”‘’—)'''
                                        ^
SyntaxError: invalid syntax

BTW if you intend to actively find and remove them, then your grep will miss the very many instances of (u"…").

friedelwolff commented 4 years ago

Thank you for the work on this. Just a general comment:

The difference from the master branch is huge, so let's restrict the changes to what is required for now. The u'' strings don't do any harm, so let's keep them for now just to make reviews easier in the next while.

I'll try to have a look at this later. Thank you for the help and reviews so far!