Python 3.5 fixes #38

clintonb · 2016-03-11T04:38:07Z

ECOM-3765

clintonb · 2016-03-11T04:39:44Z

nedbat · 2016-03-11T12:50:33Z

setup.py

@@ -51,8 +51,8 @@
    },

    install_requires=[
-        'pylint==1.4.5',
-        'pylint-django==0.6.1',
+        'pylint>=1.5.4,<2.0.0',


Do not unpin pylint. It will cause a world of hurt in edx-platform.

We need 1.5.x for Python 3.5. Hmm...how about I change this to 'pylint>=1.4.5,<2.0.0', and add pylint==1.4.5 to edx-platform? That allows for edx-platform to remain stable, and other users to get the latest compatible version. We also don't have to touch this package again just to update Pylint.

When I saw this pull request, I thought, "wait, didn't we do 3.5 already? Oh yeah, there was something that made it difficult." This is the thing. We could pin the version of pylint in edx-platform, but there's a larger problem: it's natural for pylint to add new checkers in new versions. This means that when pylint upgrades, there will likely be more violations found. Builds that depend on not exceeding some threshold of violations will then break. We don't want pin-ranges to mean that we could silently upgrade something that will break our builds. I'm not sure what to do about this.

Pinning pylint here makes this library fragile. I'd rather consuming applications, especially edx-platform, pin pylint instead.

I'm not sure how pinning makes it fragile? Part of the original goal of edx-lint was to ensure that all of our repos were linted the same. It does that mostly by using a common pylintrc, but a pinned pylint version can also help. So far, there doesn't seem to be a problem that is solved by unpinning pylint, so can we leave it pinned?

We can pin to 1.5.4.

nedbat · 2016-03-11T14:52:02Z

The tests don't pass for me locally on Python 3.5:

py35 create: /src/edx/src/edx-lint/.tox/py35
py35 installdeps: coverage
py35 inst: /src/edx/src/edx-lint/.tox/dist/edx-lint-0.5.0.zip
py35 installed: You are using pip version 7.1.2, however version 8.1.0 is available.,You should consider upgrading via the 'pip install --upgrade pip' command.,-f file:///Users/ned/Downloads/local_pypi,astroid==1.4.4,colorama==0.3.7,coverage==4.0.3,edx-lint==0.5.0,lazy-object-proxy==1.2.1,pylint==1.5.4,pylint-celery==0.3,pylint-django==0.7.1,pylint-plugin-utils==0.2.3,six==1.10.0,wheel==0.24.0,wrapt==1.10.6
py35 runtests: PYTHONHASHSEED='1755846706'
py35 runtests: commands[0] | coverage run -p -m unittest discover -b
Warning: option required-attributes is obsolete and it is slated for removal in Pylint 1.6.
.........EEEEEE
======================================================================
ERROR: test_appending_is_detected (test.test_tamper_evident.TamperEvidentFileTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 77, in test_appending_is_detected
    filename = self.write_tamper_evident(b"Am I OK?")
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 40, in write_tamper_evident
    TamperEvidentFile(filename).write(text, **kwargs)
  File "/src/edx/src/edx-lint/edx_lint/tamper_evident.py", line 37, in write
    if not text.endswith("\n"):
TypeError: endswith first arg must be bytes or a tuple of bytes, not str

======================================================================
ERROR: test_editing_is_detected (test.test_tamper_evident.TamperEvidentFileTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 86, in test_editing_is_detected
    filename = self.write_tamper_evident(b"Line 1\nLine 2\nLine 3\n")
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 40, in write_tamper_evident
    TamperEvidentFile(filename).write(text, **kwargs)
  File "/src/edx/src/edx-lint/edx_lint/tamper_evident.py", line 37, in write
    if not text.endswith("\n"):
TypeError: endswith first arg must be bytes or a tuple of bytes, not str

======================================================================
ERROR: test_hashline_formatting (test.test_tamper_evident.TamperEvidentFileTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 63, in test_hashline_formatting
    filename1 = self.write_tamper_evident(b"Hello!", hashline=b"XXX {} YYY")
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 40, in write_tamper_evident
    TamperEvidentFile(filename).write(text, **kwargs)
  File "/src/edx/src/edx-lint/edx_lint/tamper_evident.py", line 37, in write
    if not text.endswith("\n"):
TypeError: endswith first arg must be bytes or a tuple of bytes, not str

======================================================================
ERROR: test_oneline_file_is_detected (test.test_tamper_evident.TamperEvidentFileTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 96, in test_oneline_file_is_detected
    filename = self.write_tamper_evident(b"Am I OK?")
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 40, in write_tamper_evident
    TamperEvidentFile(filename).write(text, **kwargs)
  File "/src/edx/src/edx-lint/edx_lint/tamper_evident.py", line 37, in write
    if not text.endswith("\n"):
TypeError: endswith first arg must be bytes or a tuple of bytes, not str

======================================================================
ERROR: test_validating_a_good_file (test.test_tamper_evident.TamperEvidentFileTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 72, in test_validating_a_good_file
    filename = self.write_tamper_evident(b"Am I OK?")
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 40, in write_tamper_evident
    TamperEvidentFile(filename).write(text, **kwargs)
  File "/src/edx/src/edx-lint/edx_lint/tamper_evident.py", line 37, in write
    if not text.endswith("\n"):
TypeError: endswith first arg must be bytes or a tuple of bytes, not str

======================================================================
ERROR: test_writing (test.test_tamper_evident.TamperEvidentFileTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 46, in test_writing
    filename1 = self.write_tamper_evident(b"Hello!")
  File "/src/edx/src/edx-lint/test/test_tamper_evident.py", line 40, in write_tamper_evident
    TamperEvidentFile(filename).write(text, **kwargs)
  File "/src/edx/src/edx-lint/edx_lint/tamper_evident.py", line 37, in write
    if not text.endswith("\n"):
TypeError: endswith first arg must be bytes or a tuple of bytes, not str

----------------------------------------------------------------------
Ran 15 tests in 1.131s

FAILED (errors=6)
ERROR: InvocationError: '/src/edx/src/edx-lint/.tox/py35/bin/coverage run -p -m unittest discover -b'

I think the Travis builds are only running pylint?

nedbat · 2016-03-11T15:17:18Z

Locally, master passes on 2.7 and 3.5, and I am running with pylint 1.4.5. What problems were you seeing?

clintonb · 2016-03-11T15:45:33Z

Weird. This worked last night on my personal machine (3.5.?), and still works on Travis (3.5.0). It fails on my edX machine with Python 3.5.0.

nedbat · 2016-03-11T16:08:25Z

.travis.yml

-    - python: "2.7"
-      env: TOXARG="-e pylint"
+env:
+  - TOXARG="-e pylint"


This line made it so that Travis doesn't run tests, it only runs pylint.

clintonb · 2016-03-11T16:28:58Z

Backstory: I'm working on a new library, edx-drf-extensions, that needs to support Python 2.7 and 3.5. When I use edx-lint==0.4.3, I get the following errors:

pylint --rcfile=pylintrc edx_rest_framework_extensions
Traceback (most recent call last):
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/pylint/lint.py", line 910, in get_ast
    return MANAGER.ast_from_file(filepath, modname, source=True)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/manager.py", line 112, in ast_from_file
    return AstroidBuilder(self).file_build(filepath, modname)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/builder.py", line 134, in file_build
    module = self._data_build(data, modname, path)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/builder.py", line 177, in _data_build
    module = rebuilder.visit_module(node, modname, node_file, package)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 148, in visit_module
    newnode.body = [self.visit(child, newnode) for child in node.body]
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 148, in <listcomp>
    newnode.body = [self.visit(child, newnode) for child in node.body]
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 161, in visit
    return self._transform(visit_method(node, parent))
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 238, in visit_assign
    newnode.value = self.visit(node.value, newnode)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 161, in visit
    return self._transform(visit_method(node, parent))
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 331, in visit_callfunc
    if node.starargs is not None:
AttributeError: 'Call' object has no attribute 'starargs'
************* Module edx_rest_framework_extensions.authentication
F:  1, 0: <class 'AttributeError'>: 'Call' object has no attribute 'starargs' (astroid-error)
Traceback (most recent call last):
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/pylint/lint.py", line 910, in get_ast
    return MANAGER.ast_from_file(filepath, modname, source=True)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/manager.py", line 112, in ast_from_file
    return AstroidBuilder(self).file_build(filepath, modname)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/builder.py", line 134, in file_build
    module = self._data_build(data, modname, path)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/builder.py", line 177, in _data_build
    module = rebuilder.visit_module(node, modname, node_file, package)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 148, in visit_module
    newnode.body = [self.visit(child, newnode) for child in node.body]
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 148, in <listcomp>
    newnode.body = [self.visit(child, newnode) for child in node.body]
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 161, in visit
    return self._transform(visit_method(node, parent))
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 915, in visit_class
    newnode = super(TreeRebuilder3k, self).visit_class(node, parent)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 345, in visit_class
    newnode.body = [self.visit(child, newnode) for child in node.body]
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 345, in <listcomp>
    newnode.body = [self.visit(child, newnode) for child in node.body]
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 161, in visit
    return self._transform(visit_method(node, parent))
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 238, in visit_assign
    newnode.value = self.visit(node.value, newnode)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 161, in visit
    return self._transform(visit_method(node, parent))
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 331, in visit_callfunc
    if node.starargs is not None:
AttributeError: 'Call' object has no attribute 'starargs'
************* Module edx_rest_framework_extensions.tests.factories
F:  1, 0: <class 'AttributeError'>: 'Call' object has no attribute 'starargs' (astroid-error)
Traceback (most recent call last):
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/pylint/lint.py", line 910, in get_ast
    return MANAGER.ast_from_file(filepath, modname, source=True)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/manager.py", line 112, in ast_from_file
    return AstroidBuilder(self).file_build(filepath, modname)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/builder.py", line 134, in file_build
    module = self._data_build(data, modname, path)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/builder.py", line 177, in _data_build
    module = rebuilder.visit_module(node, modname, node_file, package)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 148, in visit_module
    newnode.body = [self.visit(child, newnode) for child in node.body]
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 148, in <listcomp>
    newnode.body = [self.visit(child, newnode) for child in node.body]
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 161, in visit
    return self._transform(visit_method(node, parent))
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 238, in visit_assign
    newnode.value = self.visit(node.value, newnode)
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 161, in visit
    return self._transform(visit_method(node, parent))
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/astroid/rebuilder.py", line 331, in visit_callfunc
    if node.starargs is not None:
AttributeError: 'Call' object has no attribute 'starargs'
************* Module edx_rest_framework_extensions.tests.test_authentication
F:  1, 0: <class 'AttributeError'>: 'Call' object has no attribute 'starargs' (astroid-error)
make: *** [quality] Error 1

I have no issues when I manually update Pylint from 1.4.5 to 1.5.4. That's the actual issue I need to resolve here.

I'm not sure what happened last night with the bytes vs. string issue, prompting me to update that code; but, it is no longer an issue. I am reverting those changes now.

clintonb · 2016-03-11T17:18:12Z

I've found the disconnect. The tests certainly work; however, in practice edx_lint write pylintrc fails with the following error:

(edx-drf-extensions)Clinton Blackburn's MacBook Pro:edx-drf-extensions cblackburn$ edx_lint write pylintrc
Checking existing copy of pylintrc
Reading edx_lint/files/pylintrc
Applying local tweaks from pylintrc_tweaks
Writing pylintrc
Traceback (most recent call last):
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/bin/edx_lint", line 9, in <module>
    load_entry_point('edx-lint==0.5.0', 'console_scripts', 'edx_lint')()
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/edx_lint/cmd/main.py", line 23, in main
    return write_main(argv[1:])
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/edx_lint/cmd/write.py", line 126, in write_main
    out_tef.write(str(output_text.getvalue()))
  File "/Users/cblackburn/.virtualenvs/edx-drf-extensions/lib/python3.5/site-packages/edx_lint/tamper_evident.py", line 37, in write
    if not text.endswith(b"\n"):
TypeError: endswith first arg must be str or a tuple of str, not bytes

ECOM-3765

nedbat · 2016-03-11T19:03:02Z

edx_lint/tamper_evident.py

@@ -34,6 +34,10 @@ def write(self, text, hashline=b"# {}"):
                to the file, with "{}" replaced with the hash.

        """
+        # Ensure we are working with bytes.
+        if isinstance(text, str):
+            text = str.encode(text)


This seems wrong for py2, where we expect a "str", and the .encode will do the wrong thing for non-ascii. The argument is documented as a bytestring, so why do we need these lines?

The argument being passed when running the command is of type str: https://github.com/edx/edx-lint/blob/master/edx_lint/cmd/write.py#L123. Perhaps a better solution is to update this method to accept str instead of bytes?

Or remove the str from that line in write.py

clintonb · 2016-03-11T20:59:21Z

test/test_cmd.py

+        """
+        return main.main(argv)
+
+    @unittest.skip('Default test runners have issues finding the file.')


Note that I've added a test that is broken for everything but PyCharm. Still trying to figure out why.

#39 filed to address this issue in the future.

ECOM-3765

clintonb · 2016-03-14T17:26:22Z

@nedbat let me know if you have additional feedback, or if we are good to merge.

clintonb · 2016-03-14T22:04:17Z

@nedbat are we good to merge?

nedbat · 2016-03-15T15:38:06Z

I have a feeling of dread about this, but I don't have a better solution, so 👍

clintonb · 2016-03-15T15:38:50Z

🙊

Python 3.5 fixes

clintonb force-pushed the clintonb/python-3.5 branch from 2f14ce4 to 24b03b0 Compare March 11, 2016 04:46

nedbat reviewed Mar 11, 2016
View reviewed changes

clintonb force-pushed the clintonb/python-3.5 branch from 24b03b0 to b90a960 Compare March 11, 2016 16:31

clintonb mentioned this pull request Mar 11, 2016

add edx-management-commands dependency. openedx/edx-platform#11794

Merged

clintonb force-pushed the clintonb/python-3.5 branch from b99e9b1 to f1a7092 Compare March 11, 2016 18:49

Added myself to AUTHORS

645881d

ECOM-3765

clintonb force-pushed the clintonb/python-3.5 branch 2 times, most recently from 78d4f01 to b66f228 Compare March 11, 2016 18:53

nedbat reviewed Mar 11, 2016
View reviewed changes

clintonb force-pushed the clintonb/python-3.5 branch 4 times, most recently from 22f3821 to 0e0d183 Compare March 11, 2016 20:29

clintonb reviewed Mar 11, 2016
View reviewed changes

clintonb mentioned this pull request Mar 11, 2016

Added support for publication to PyPI edx/edx-django-extensions#2

Merged

clintonb and others added 4 commits March 14, 2016 13:12

Updated requirements

b0cae1b

ECOM-3765

Added test for the command that writes pylintrc files

1d13550

ECOM-3765

Testing with Python 3.5 on Travis

ce56d5f

ECOM-3765

Updated version

26cbe50

ECOM-3765

clintonb force-pushed the clintonb/python-3.5 branch from 0e0d183 to 26cbe50 Compare March 14, 2016 17:12

clintonb mentioned this pull request Mar 14, 2016

Fix test_pylintrc_file_created #39

Closed

clintonb added a commit that referenced this pull request Mar 15, 2016

Merge pull request #38 from edx/clintonb/python-3.5

7ada9bd

Python 3.5 fixes

clintonb merged commit 7ada9bd into master Mar 15, 2016

clintonb deleted the clintonb/python-3.5 branch March 15, 2016 15:39

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Python 3.5 fixes #38

Python 3.5 fixes #38

clintonb commented Mar 11, 2016

clintonb commented Mar 11, 2016

nedbat Mar 11, 2016

clintonb Mar 11, 2016

nedbat Mar 11, 2016

clintonb Mar 11, 2016

nedbat Mar 14, 2016

clintonb Mar 14, 2016

nedbat commented Mar 11, 2016

nedbat commented Mar 11, 2016

clintonb commented Mar 11, 2016

nedbat Mar 11, 2016

clintonb commented Mar 11, 2016

clintonb commented Mar 11, 2016

nedbat Mar 11, 2016

clintonb Mar 11, 2016

nedbat Mar 11, 2016

clintonb Mar 11, 2016

clintonb Mar 14, 2016

clintonb commented Mar 14, 2016

clintonb commented Mar 14, 2016

nedbat commented Mar 15, 2016

clintonb commented Mar 15, 2016

Python 3.5 fixes #38

Python 3.5 fixes #38

Conversation

clintonb commented Mar 11, 2016

clintonb commented Mar 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nedbat commented Mar 11, 2016

nedbat commented Mar 11, 2016

clintonb commented Mar 11, 2016

Choose a reason for hiding this comment

clintonb commented Mar 11, 2016

clintonb commented Mar 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clintonb commented Mar 14, 2016

clintonb commented Mar 14, 2016

nedbat commented Mar 15, 2016

clintonb commented Mar 15, 2016