Python 3 compatibility #7

lucaswiman · 2016-09-14T02:03:47Z

@gorakhargosh cc @nickjacobson Supercedes #6. Fixes #4.

Adds a tox.ini file for running the tests in both python 2 and python 3.
Updates the raised ValueError to something with the same __repr__ in both python 2 and python 3. This fixes the doctests.
Updates trove classifiers to indicate this package is python 2 and python 3 compatible. Bumps version to 0.2.0.

Other than some of the build scripts (which I couldn't get working even on python 2), everything was Python 3 compatible, except that the doctests had an annoying incompatibility where the exception had a different repr between python 2 and python 3:

Failed example:
    match_path("/users/gorakhargosh/FOOBAR.PY", ["*.py"], ["*.PY"], False)
Expected:
    Traceback (most recent call last):
        ...
    ValueError: conflicting patterns `set(['*.py'])` included and excluded
Got:
    Traceback (most recent call last):
      File "/Users/lucaswiman/.pyenv/versions/3.5/lib/python3.5/doctest.py", line 1320, in __run
        compileflags, 1), test.globs)
      File "<doctest pathtools.patterns.match_path[5]>", line 1, in <module>
        match_path("/users/gorakhargosh/FOOBAR.PY", ["*.py"], ["*.PY"], False)
      File "/Users/lucaswiman/opensource/pathtools/pathtools/patterns.py", line 174, in match_path
        return _match_path(pathname, included, excluded, case_sensitive)
      File "/Users/lucaswiman/opensource/pathtools/pathtools/patterns.py", line 125, in _match_path
        % common_patterns)
    ValueError: conflicting patterns `{'*.py'}` included and excluded

I updated the error so it includes a list of conflicting patterns. That has the same repr on python 2 and python 3.

- Adds a tox.ini file for running the tests in both python 2 and python 3. - Update raised ValueError to something with the same __repr__ in both python 2 and python 3. This fixes the doctests. - Update trove classifiers to indicate this package is python 2 and python 3 compatible.

lucaswiman · 2016-09-14T02:05:49Z

scripts/bootstrap.py

@@ -69,12 +69,12 @@
 except ImportError:
    ez = {}
    if USE_DISTRIBUTE:
-        exec urllib2.urlopen('http://python-distribute.org/distribute_setup.py'
-                         ).read() in ez
+        exec(urllib2.urlopen('http://python-distribute.org/distribute_setup.py'


These fixes were added automatically by the futurize command. The script still isn't quite python 3 compatible, but seems to be broken anyway, since the link 404s.

Has upstream bootstrap been updated for python 3 as well?

It seems to have been updated, yes: https://raw.githubusercontent.com/buildout/buildout/master/bootstrap/bootstrap.py I don't understand the build scripts well enough to test them, so I've reverted my changes to the scripts directory in 27ce639.

My primary purpose in this PR was get the test suite to pass on Python 3 and update the trove classifiers so that pathtools no longer appears as "not ready" on the wall of python 3 readiness. The build scripts can stay on python 2 until its end of life.

hugovk

It'd be great to run the tests on Travis CI.

eirnym · 2018-03-15T11:07:14Z

@lucaswiman Why have you reverted changes in scripts/*.py? Python 2.7 supports these calls natively.

eirnym · 2018-03-15T11:15:39Z

@hugovk It's a separate task to run tests on Travis CI. I'd prefer to be sure that this task is complete and this PR doesn't add new features.

lucaswiman · 2018-03-15T17:53:15Z

Why have you reverted changes in scripts/*.py? Python 2.7 supports these calls natively.

As I said in my comment above, the scripts still aren't python 3 compatible after those changes, but I don't understand scripts well enough to test them on python 3. They're not part of the sdist package, so upgrading them isn't as big a priority.

I can investigate making additional changes if you want. However, I do not want to devote more effort unless there's some chance it will get merged and deployed to pypi. This PR has been open for 18 months. Are you in favor of deploying this change to pypi once your review comments are addressed?

wiggin15 · 2018-10-06T18:49:18Z

pathtools/path.py

@@ -73,7 +73,7 @@ def walk(path, topdown=topdown, followlinks=followlinks):
            try:
                yield next(os.walk(path, topdown=topdown, followlinks=followlinks))
            except NameError:
-                yield os.walk(path, topdown=topdown, followlinks=followlinks).next() #IGNORE:E1101
+                yield next(os.walk(path, topdown=topdown, followlinks=followlinks)) #IGNORE:E1101


Note that this is now exactly the same line as inside the "try". I think this was here for Python 2.6 compatibility. Instead of this change, the except clause should simply be removed.

westover · 2019-07-30T21:07:37Z

With Python2 EOL approaching I am curious if this effort can be revived

eirnym · 2019-07-30T21:29:27Z

Why have you reverted changes in scripts/*.py? Python 2.7 supports these calls natively.

As I said in my comment above, the scripts still aren't python 3 compatible after those changes, but I don't understand scripts well enough to test them on python 3. They're not part of the sdist package, so upgrading them isn't as big a priority.

I can investigate making additional changes if you want. However, I do not want to devote more effort unless there's some chance it will get merged and deployed to pypi. This PR has been open for 18 months. Are you in favor of deploying this change to pypi once your review comments are addressed?

This could be done in a few steps. It's not required to do it all at once. For bootstrap on python3 I'd recommend to install the most recent setuptools, wheel and pip if you really had to do bootstrapping (I really doubt in it).

lucaswiman · 2019-07-30T21:32:41Z

I am not planning any further work on this. Please open a new PR if anyone wants to take this on.

eirnym · 2019-07-31T11:23:20Z

@lucaswiman Thank you for your efforts

Lucas Wiman added 3 commits September 13, 2016 18:40

git ls-files | grep [.]py | xargs futurize --stage1 -w

6734124

Update version string to 0.2.0 for deployment to pypi.

64cca47

lucaswiman reviewed Sep 14, 2016
View reviewed changes

Revert chagnes to scripts/*.py

27ce639

hugovk approved these changes Oct 14, 2017

View reviewed changes

wiggin15 reviewed Oct 6, 2018

View reviewed changes

lucaswiman closed this Jul 30, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python 3 compatibility #7

Python 3 compatibility #7

lucaswiman commented Sep 14, 2016

lucaswiman Sep 14, 2016

gorakhargosh Oct 6, 2016

lucaswiman Jul 22, 2017

hugovk left a comment

eirnym commented Mar 15, 2018

eirnym commented Mar 15, 2018

lucaswiman commented Mar 15, 2018 •

edited

wiggin15 Oct 6, 2018

westover commented Jul 30, 2019

eirnym commented Jul 30, 2019

lucaswiman commented Jul 30, 2019

eirnym commented Jul 31, 2019

Python 3 compatibility #7

Python 3 compatibility #7

Conversation

lucaswiman commented Sep 14, 2016

lucaswiman Sep 14, 2016

Choose a reason for hiding this comment

gorakhargosh Oct 6, 2016

Choose a reason for hiding this comment

lucaswiman Jul 22, 2017

Choose a reason for hiding this comment

hugovk left a comment

Choose a reason for hiding this comment

eirnym commented Mar 15, 2018

eirnym commented Mar 15, 2018

lucaswiman commented Mar 15, 2018 • edited

wiggin15 Oct 6, 2018

Choose a reason for hiding this comment

westover commented Jul 30, 2019

eirnym commented Jul 30, 2019

lucaswiman commented Jul 30, 2019

eirnym commented Jul 31, 2019

lucaswiman commented Mar 15, 2018 •

edited