Why is this removed?

unused var

np.linspace(0, 2*np.pi, nverts) would probably make this a lot clearer - @tacaswell WDYT ?

Didn't a bunch of these files get changed during the SciPy sprints a few
months ago? Particularly with respect to spacing around operators? Or was
that in the codebase itself?

Also, now that these files have been PEP8-ified, I think there is some sort
of white-list or black-list in the testing directory that indicates which
files should be tested for pep8-compliance. This way the files stay pep8-ed.

On Thu, Aug 28, 2014 at 9:09 AM, Thomas Hisch [email protected]
wrote:

In examples/api/collections_demo.py:

@@ -27,10 +27,10 @@

Make some spirals

r = np.array(range(nverts))
-theta = np.array(range(nverts)) * (2_np.pi)/(nverts-1)
+theta = np.array(range(nverts))_(2*np.pi)/(nverts - 1)

np.linspace(0, 2*np.pi, nverts) would probably make this a lot clearer -
@tacaswell https://github.com/tacaswell WDYT ?

—
Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/3425/files#r16837317.

👍 to both comments.

@WeatherGod are you referring to the blacklist in the test_coding_standards.py file?

Ah, yes. So, a quick read through that file, it seems to me that we don't
pep8 test the example files at all, I think? Should we?

On Thu, Aug 28, 2014 at 9:52 AM, Thomas Hisch [email protected]
wrote:

In examples/api/collections_demo.py:

@@ -27,10 +27,10 @@

Make some spirals

r = np.array(range(nverts))
-theta = np.array(range(nverts)) * (2_np.pi)/(nverts-1)
+theta = np.array(range(nverts))_(2*np.pi)/(nverts - 1)

@WeatherGod https://github.com/WeatherGod are you referring to the
blacklist in the test_coding_standards.py file?

—
Reply to this email directly or view it on GitHub
https://github.com/matplotlib/matplotlib/pull/3425/files#r16839843.

Yes. I added a simple implementation in test_coding_standards.py. Let's see if it works with travis CI.

only add kwargs at the end to maintain back-compatibility.

spurious blank line

Is this still under 80 characters (can't tell with the github viewer)?

yes: 78 chars

BTW, the line length of the examples gets checked in the test_coding_standards unit test

ah, good.

I guess the autopep8 tool got fatigued and gave up removing the spaces for these kwargs...

I can't reproduce it. Probably I messed it up.

edit: looked into the wrong line. It is indeed a problem of autopep8 - created hhatto/autopep8#158

still have extra spaces around the "axes_pad" and "label_mode" kwargs

I personally prefer NO spaces around the arithmetic operator in this case. I'll revert this change if you are fine with that

I'm not going to change this as I found occurrences of indexing expressions in the matplotlib src where spaces around operators are both used (type1font.py) and not used (tri/triinterpolate.py).

It seems to be an idiosyncrasy of the particular pep8 tool that it always puts spaces around plus and minus, and never allows them around asterisk and solidus. This is completely contrary to the letter and spirit of the actual PEP 8. Nevertheless, it seems we are stuck with it because the advantage of automated checking is deemed to outweigh the disadvantage of the occasional poor choice made by the automated tools. (In the line above, having spaces around the second asterisk would slightly enhance readability.)

The spaces were removed by me on purpose and not by the (auto)pep8 tool. I'll readd the spaces around the second asterisk in a coming PR

@Thisch I don't want to be too fussy--many things are a matter of taste, and not worth fiddling with--but if you are making extensive changes to improve style and make things more consistent with mpl guidelines, then you could also try to eliminate the use of trailing backslashes as line continuations. Methods include the use of parentheses to make a list of imports, or breaking an import up into multiple imports; use of a temporary variable for a chunk of an expression that is otherwise getting out of hand; and taking advantage of whatever form of parentheses or brackets is appropriate.

@efiring thx for your comment. I care a lot about consistency and therefore want to help making the mpl codebase more consistent. There already exists a tool to break a list of imports into multiple imports called isort (https://github.com/timothycrosley/isort - check out its Readme file). Automating fixing your other mentioned issues is probably not worth the effort even if I fully agree to your comments.

Interesting, but I think isort is probably overkill for us, and would result in unproductive code churn. I think our imports are mostly OK. (If it were to be used, my configuration choices would include "multi-line-output=0".) I sympathize with the author's explanation at the end: "I'm too lazy to spend 8 hours mindlessly performing a function, but not too lazy to spend 16 hours automating it."