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

Updated coding standards test to raise an exception containing the PEP8 failiures. #2099

Merged
merged 2 commits into from Sep 19, 2013

Conversation

pelson
Copy link
Member

@pelson pelson commented May 31, 2013

Closes #2091

@pelson
Copy link
Member Author

pelson commented Jun 3, 2013

Good for merge?

@mdboom
Copy link
Member

mdboom commented Jun 3, 2013

Before merging, I'd like to test this right in Travis by introducing some PEP8 errors and making sure the output appears in the pep8 test's output section and not up above.

@pelson
Copy link
Member Author

pelson commented Jun 3, 2013

Added a commit on top of 887d162

@pelson
Copy link
Member Author

pelson commented Jun 3, 2013

I'll remove the commit when we've seen the travis result.

@mdboom
Copy link
Member

mdboom commented Jun 4, 2013

The test seems to be passing (even with your bad commit)...

@@ -179,7 +179,7 @@ def byte2str(b): return b
raise ImportError('matplotlib requires Python 2.4 or later')


import numpy
import numpy # This is a sneaky little comment. It really doesn't comply with the PEP8 guidelines...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of curiosity, what was that for? I almost wrote that this wasn't pep8 before reading the comment...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see earlier comments. I wanted to introduce some PEP8 test failures.

@mdboom
Copy link
Member

mdboom commented Sep 17, 2013

@pelson: Should we revive this? First it needs a rebase, then we need to confirm that the bad PEP8 commit shows up in the Travis log.

@pelson
Copy link
Member Author

pelson commented Sep 18, 2013

Sure thing. I'll do what I can this afternoon.

@pelson
Copy link
Member Author

pelson commented Sep 18, 2013

Ok. Rebased. Hopefully I've left enough PEP8 failures in there to demonstrate it's working. I'll then fix those intentional (ahem) PEP8 problems. 😄

@pelson
Copy link
Member Author

pelson commented Sep 19, 2013

Great. The travis build failed: https://s3.amazonaws.com/archive.travis-ci.org/jobs/11514742/log.txt
In short:

...
matplotlib.tests.test_transforms.test_pre_transform_plotting.test ... ok
matplotlib.tests.test_transforms.test_pre_transform_plotting.test ... ok
matplotlib.tests.test_transforms.test_pre_transform_plotting.test ... ok
matplotlib.tests.test_subplots.test_subplots ... ok

======================================================================
FAIL: matplotlib.tests.test_coding_standards.test_pep8_conformance
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py", line 245, in test_pep8_conformance
    '\n'.join(reporter._global_deferred_print))))
AssertionError: Found code syntax errors (and warnings):
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py:144:1: W293 blank line contains whitespace
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py:147:1: W293 blank line contains whitespace
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py:150:1: W293 blank line contains whitespace
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py:152:80: E501 line too long (82 > 79 characters)
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py:155:1: W293 blank line contains whitespace
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py:156:80: E501 line too long (83 > 79 characters)
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py:157:80: E501 line too long (81 > 79 characters)
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_coding_standards.py:171:1: W293 blank line contains whitespace
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_table.py:10:14: E201 whitespace after '['
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_table.py:10:30: E231 missing whitespace after ','
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_table.py:11:14: E201 whitespace after '['
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_table.py:11:30: E231 missing whitespace after ','
/home/travis/virtualenv/python2.7/local/lib/python2.7/site-packages/matplotlib-1.3.0-py2.7-linux-x86_64.egg/matplotlib/tests/test_table.py:17:5: E303 too many blank lines (2)

----------------------------------------------------------------------
Ran 1462 tests in 164.792s

FAILED (KNOWNFAIL=2, SKIP=5, failures=1)

I'll go ahead and tidy up these two failures, then I think we can go ahead and merge this in the next couple of hours @mdboom.

@mdboom
Copy link
Member

mdboom commented Sep 19, 2013

Thanks.

mdboom added a commit that referenced this pull request Sep 19, 2013
Updated coding standards test to raise an exception containing the PEP8 failiures.
@mdboom mdboom merged commit 42165c8 into matplotlib:v1.3.x Sep 19, 2013
@pelson pelson deleted the re-enable_pep8_test branch September 20, 2013 10:44
@mdboom
Copy link
Member

mdboom commented Sep 20, 2013

Thanks -- I think this is working well on the 1.3.x branch. Unfortunately, on master, we have a whole bunch of PEP8 problems that have crept in. I think I'm just going to disable this temporarily over there to get us through the 1.3.1 release (to lessen the possibility of noise as I merge things back into master), and then resolve the PEP8 issues properly after that. I have created #2443 as a reminder.

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.

None yet

3 participants