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

Python 3 compatibility issues #401

Closed
cclauss opened this issue Apr 23, 2019 · 10 comments
Closed

Python 3 compatibility issues #401

cclauss opened this issue Apr 23, 2019 · 10 comments

Comments

@cclauss
Copy link
Contributor

cclauss commented Apr 23, 2019

flake8 testing of https://github.com/RobotWebTools/rosbridge_suite on Python 3.7.1

$ flake8 . --count --select=E9,F63,F72,F82 --show-source --statistics

./rosbridge_library/src/rosbridge_library/util/cbor.py:152:32: F821 undefined name 'unicode'
        return isinstance(val, unicode)
                               ^
./rosbridge_library/src/rosbridge_library/util/cbor.py:218:36: F821 undefined name 'basestring'
        return isinstance(x, (str, basestring, bytes, unicode))
                                   ^
./rosbridge_library/src/rosbridge_library/util/cbor.py:218:55: F821 undefined name 'unicode'
        return isinstance(x, (str, basestring, bytes, unicode))
                                                      ^
./rosbridge_library/src/rosbridge_library/util/cbor.py:220:36: F821 undefined name 'long'
        return isinstance(x, (int, long))
                                   ^
./rosbridge_library/src/rosbridge_library/util/cbor.py:371:18: F821 undefined name 'xrange'
        for i in xrange(aux):
                 ^
./rosbridge_library/src/rosbridge_library/util/cbor.py:378:18: F821 undefined name 'xrange'
        for i in xrange(aux):
                 ^
./rosbridge_library/src/rosbridge_library/util/__init__.py:16:26: F821 undefined name 'unicode'
    string_types = (str, unicode)
                         ^
./rosbridge_library/src/rosbridge_library/internal/cbor_conversion.py:49:20: F821 undefined name 'unicode'
            slot = unicode(slot)
                   ^
./rosbridge_library/src/rosbridge_library/internal/cbor_conversion.py:53:25: F821 undefined name 'unicode'
            out[slot] = unicode(val) if PYTHON2 else str(val)
                        ^
./rosbridge_library/src/rosbridge_library/internal/message_conversion.py:70:35: F821 undefined name 'long'
    primitive_types = [bool, int, long, float]
                                  ^
./rosbridge_library/src/rosbridge_library/capabilities/subscribe.py:316:21: F821 undefined name 'unicode'
            topic = unicode(topic)
                    ^
./rosbridge_library/test/internal/test_message_conversion.py:21:26: F821 undefined name 'unicode'
    string_types = (str, unicode)
                         ^
./rosbridge_library/test/internal/test_services.py:19:26: F821 undefined name 'unicode'
    string_types = (str, unicode)
                         ^
./rosbridge_library/test/internal/test_services.py:30:53: F821 undefined name 'unicode'
    elif sys.version_info < (3,0) and isinstance(d, unicode):
                                                    ^
./rosbridge_library/test/internal/test_services.py:31:16: F821 undefined name 'unicode'
        return unicode(random.random())
               ^
./rosbridge_library/test/internal/test_cbor_conversion.py:39:55: F821 undefined name 'unicode'
            self.assertEqual(type(extracted['data']), unicode)
                                                      ^
./rosbridge_library/test/internal/test_cbor_conversion.py:152:45: F821 undefined name 'unicode'
                self.assertEqual(type(key), unicode)
                                            ^
./rosapi/src/rosapi/params.py:70:8: F632 use ==/!= to compare str, bytes, and int literals
    if default is not "":
       ^
1     F632 use ==/!= to compare str, bytes, and int literals
17    F821 undefined name 'unicode'
18

E901,E999,F821,F822,F823 are the "showstopper" flake8 issues that can halt the runtime with a SyntaxError, NameError, etc. These 5 are different from most other flake8 issues which are merely "style violations" -- useful for readability but they do not effect runtime safety.

  • F821: undefined name name
  • F822: undefined name name in __all__
  • F823: local variable name referenced before assignment
  • E901: SyntaxError or IndentationError
  • E999: SyntaxError -- failed to compile a file into an Abstract Syntax Tree
@mvollrath
Copy link
Contributor

I've reviewed all of the CBOR script related warnings, they are all in Python 2 compatibility blocks. Do we need to scrub all Python 2 support?

@cclauss
Copy link
Contributor Author

cclauss commented Jul 2, 2019

The linters will get confused by code that does not follow the Python porting best practice use feature detection instead of version detection so if you are sure that the F821 issues are all safe uses done via version detection then those issues can be ignored. The usual approaches are to add the six module and replace unicode with six.text_type or without six, to do it like:

try:
    long
    unicode
except NameError:  # Python 3
    long = int
    unicode = str

@mikepurvis
Copy link

Is anyone actively working on this? I'd like a version of rosbridge that is either bilingual or ported to Python 3.

@mvollrath
Copy link
Contributor

I've been using rosbridge with Python 3, but have not tested the latest release with it. It would be prudent to start running the tests in both 2 and 3.

@cclauss
Copy link
Contributor Author

cclauss commented Jul 17, 2019

#420 runs basic linting on both Python 2 and Python 3.

@mikepurvis
Copy link

@mvollrath Great, thanks for the confirmation; I've done some basic testing under Python 3 on my system and it does indeed seem to be working well.

jihoonl pushed a commit that referenced this issue Jul 18, 2019
* Travis CI: Look for Python syntax errors and undefined name

_It would be prudent to start running the tests in both 2 and 3._  #401 (comment)

* Add names to protect the guilty

* Five jobs, not six

* Identity is not the same thing as equality in Python

* Flake8 tests now pass on Python 2
@github-actions
Copy link

This issue has been marked as stale because it has been open for 180 days with no activity. Please remove the stale label or add a comment to keep it open.

@github-actions github-actions bot added the stale label Aug 18, 2021
@cclauss
Copy link
Contributor Author

cclauss commented Aug 18, 2021

These issues continue to grow. #592 Demonstrates that we are now at 26 undefined names on Python 3.

Does this codebase still need to support Python 2? It would be helpful if README.md stated what versions of Python are supported.

@mvollrath
Copy link
Contributor

ROS1 melodic requires Python 2 support until May 2023 and noetic requires Python 3 until May 2025. ROS2 branch does not require any Python 2 support.

@github-actions github-actions bot removed the stale label Aug 19, 2021
@amacneil
Copy link
Member

  • ros2 branch has now been ported to python 3 and warnings fixed (thanks @cclauss!)
  • ros1 branch supports both python 2 and 3, but is in maintenance mode and will only receive critical bug fixes

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

No branches or pull requests

4 participants