Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Python 3 compatibility, fix #888 #889

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

nim65s
Copy link

@nim65s nim65s commented Nov 28, 2018

Hi again,

This PR follows enki-community/enki#61 & enki-community/enki#62 and fixes #888

As in enki-community/enki#62, to ensure that there is no syntax errors in python scripts neither in python2 nor 3, I used the flake8 linter with python2 -m flake8 and python3 -m flake8.

I also ran the full test suite with success with both python versions.

Most changes are:

  • whitespaces, around operators, after a #, and some tabs have been replaced by spaces. To see this PR more clearly, you can ignore this by adding ?w=1 at the end of the url
  • using the print function
  • removing unused imports
  • splitting lines that are too long
  • changing if var == True: into if var:
  • changing if var {!,=}= None into if var is {not,} None
  • using six for raw_input and basestring
  • relatively importing maintainer/translations/path.py, and removing the wildcard import

isort and yapf have also been helping a bit, and they have made aesthetic choices you might not be comfortable with. If this is the case, I can obviously revert that to the previous format that you did prefer.

NB: u-strings have not been removed, therefore this will not work on python3 < 3.3, but most distributions provide >= 3.4.

PS: This is a Work In Progress, as there is no point in merging this PR before those that enable python3 build in enki

@nim65s
Copy link
Author

nim65s commented Nov 28, 2018

Also, this adds a dependency on six. We could either hope that it is already widely available everywhere, or document that, or just include it in the project:

It is contained in only one Python file, so it can be easily copied into your project. (The copyright and license notice must be retained.)

@stephanemagnenat
Copy link
Member

Thank you for your PR!

As in Enki, this PR contains both Python 3 compatibility code and whitespace and other aesthetic changes. Using tabs instead of whitespace is the Aseba style. Similarly, keeping relatively long lines is as well, as most editors have good soft warp. At that point, I do not want to change this style as it has been working well for a long time, and re-indenting everything would both loose the blame history.

Can you please submit a PR that just bring the necessary changes for Python 3 compatibility? Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Python3 compatibility
2 participants