nexedi / erp5

Let's see if it works first

python2 does not support the "raise with_traceback" syntax ( https://github.com/python/cpython/blob/2.7/Lib/lib2to3/fixes/fix_raise.py#L83 )

added 8 commits

• 1e3f897f - 1 commit from branch nexedi:master
• 646b86bc - *: refactor with 2to3's except fixer
• 4e5ad574 - *: adjust what 2to3's except fixer left as TODO
• 1c60256f - *: refactor with a modified 2to3's raise fixer
• 1cb15442 - *: adjust what 2to3's raise fixer could not handle automatically
• 6d0d0fcb - *: refactor with 2to3's ne fixer
• f203045c - *: refactor with a modified 2to3's numliterals fixer
• 7681b1d7 - *: use a str for version

Compare with previous version

unmarked as a Work In Progress

changed title from Use python3 compatible syntax to Use python3 compatible syntax where it's already possible

The background is a bit silly. These days, I'm using a editor with an embedded a python3 pyflakes and it complains when the files are using some syntax that's only valid in python2.

I started to make some trivial fixes to make the code syntaxically correct on python3 while keeping the same behavior on python2 ( for example d8999426 ). After discussing with @georgios.dagkakis we thought that if I do this, it would be better to change all files at a time instead of making several small commits.

Of course I can fix my editor to check code as python2, but I guess it's the right direction to start using code that's compatible with python2 and python3 already. According to tests, this looks OK.

@jm @bminusl is this interfering with some other plans of python3 compatibility ?

Thanks @jerome . To me it looks good if tests look good. I think my complaint was that I saw it in some unrelated merge request, so I got confused and said it would be better to do it at once. So if others are ok, for me it is why not now?

I find this work somewhat premature in that people will still commit with invalid py3 syntax. More generally, it's like a bugfix without unit tests. We should have one checking the syntax of all *.py in products/bt5.

After discussing with @georgios.dagkakis we thought that if I do this, it would be better to change all files at a time instead of making several small commits.

Yes, but let's exclude erp5/util in order not to conflict with @bminusl work.

There are many changes like

-  raise Foo, (bar,)
+  raise Foo(bar,)

I wish it is raise Foo(bar)

About the conversion from except X, T to except X as T, do you know neoppod@0d36de7b ? We should change 2to3 to report if the variable receiving the except value is accessed outside the except clause and check manually those cases.

That's ugly.

because of the \ now useless inside (...) ?

Ugly indeed, but I think original code is ugly here also

mentioned in merge request slapos!509

do you know neoppod@0d36de7b ?

I did not know, so it's very different in python3.

try:
    raise ValueError("ahah")
except ValueError as e:
    print (e)
print (e)

$ python2 repro.py
ahah
ahah

$ python3 repro.py
ahah
Traceback (most recent call last):
  File "repro.py", line 5, in <module>
    print (e)
NameError: name 'e' is not defined

In the case of neo it's something like this:

from __future__ import print_function
try:
    raise ValueError("ahah")
except ValueError as e:
    print ("1", e)
    try:
       raise ValueError("ohoh")
    except ValueError as e:
       print ("2", e)
    print ("3", e)

$ python2 repro2.py 
1 ahah
2 ohoh
3 ohoh

$ python3 repro2.py 
1 ahah
2 ohoh
Traceback (most recent call last):
  File "repro2.py", line 3, in <module>
    raise ValueError("ahah")
ValueError: ahah

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "repro2.py", line 10, in <module>
    print ("3", e)
NameError: name 'e' is not defined

but it's another case of "use the exception variable outside of except block", the second except block in that case. It might be possible to detect this in 2to3, it seems powerful.

marked as a Work In Progress

About this variables in except clause, it's worth mentioning that pyflakes detects it in the first case ( repro.py:5: undefined name 'e' ) and that they have an open issue for the second case. Pylint does not detect anything ( https://github.com/PyCQA/pylint/issues/626 ).

About the code style, if we were using an automatic code formatting (such as yapf or blake), this would not be an issue.

( I don't think I'll work more on this soon, but I wanted to mention that pyflakes can help here )

Something based on these 2to3 checks could be integrated in our existing "coding style" suite, so that we detect python2 only syntaxs, lib2to3 makes it easy:

  import lib2to3.refactor
  refactoring_tool = lib2to3.refactor.RefactoringTool(
        fixer_names=('lib2to3.fixes.fix_except',  # add more fixers, we can even add our owns
  ))
  new_code = str(refactoring_tool.refactor_string(code, script_name))
  if new_code != code:
    i = 0
    for new_line, old_line in zip(new_code.splitlines(), code.splitlines()):
      i += 1
      if new_line != old_line:
        message_list.append(
          dict(row=i, column=0, type='W', text='-{}\n+{}'.format(old_line, new_line)))

[ edit: this snippet was to be added in extension.erp5.ERP5Administration ]

This would also integrated in editor: