Skip to content
This repository

remove all trailling spaces #837

Merged
merged 1 commit into from over 2 years ago

4 participants

Bernardo B. Marques Thomas Kluyver Min RK gabriellima
Bernardo B. Marques

I ran a script to remove all trailling spaces from the project source.
I know IPython is big project, so is hard to keep the code clean.
I noticied a looot of trailling spaces, and unused imports, PEP8 mistakes...

here are some tips:
-Tralling spaces: you can run this code in the folder to remove all trailling spaces:

for f in $(find -name *.py | xargs egrep -r -l '\s+$') ; do
sed -r -i 's/\s+$//' $f ;
done

-Unused imports: If you use VIM, you can use the plugin "PyFlakes" which tell you when a import isn't being used and other things..

-PEP8: you can use this tool to check your code: https://github.com/jcrocholl/pep8

Thanks.
PS. IPython is great =]

Thomas Kluyver
Collaborator

Thanks, Bernardo.

Tip to anyone else looking at this: the diff view is big enough that it slows my browser to a crawl. I've done a diff with the --ignore-space-at-eol option, which confirms that there's no other changes in this, so I'll go ahead and merge it.

Thomas Kluyver takluyver merged commit 34c1043 into from October 04, 2011
Thomas Kluyver takluyver closed this October 04, 2011
Thomas Kluyver
Collaborator

I just had to put a few trailing spaces back into some doctests, which failed because there was supposed to be trailing spaces:

90f40a6

Bernardo, if you install IPython, you should get an iptest script which will run all the automatic tests. You can do this in a virtualenv to keep development versions separate from your main IPython installation. As you can see, even the most apparently harmless changes can lead to tests failing.

Min RK
Owner

Sorry, I guess I was too slow, here. I would have suggested we not merge this PR. We most certainly DO NOT enforce pep8 in IPython.

Bernardo B. Marques

This PR was not about PEP8, it was about trailling spaces. And I'm sorry for the doctests.. my bad =P

Thomas Kluyver
Collaborator

Or maybe I was too fast. At least a bit of whitespace cleanup shouldn't cause any problems.

Min RK
Owner

You are right, there shouldn't be any real problems caused by this, only a few inconveniences - it will probably conflict with a few outstanding PRs, and it slightly reduces the effectiveness of a few tools, like blame, diff, and log, but in all shouldn't be a big deal.

I should note that we do follow pep8 mostly (with the notable exception of its ludicrous 72/79-char line-lengths). What we do not do is enforce any kind of strict coding standard that could result in mass 'cleanup' commits.

Thomas Kluyver
Collaborator

Hmmm, sorry, I overlooked the potential conflict with other PRs. At a quick glance, it looks like several will need rebasing, unfortunately.

Thomas Kluyver
Collaborator

I've rebased my own pull requests. I'll volunteer to rebase anyone other that are in conflict with this.

gabriellima

I'm following this issue, and yes, this is a common mistake.
But, looking at the last commit by @takluyver to fix mistakes, I think that they could also be fixed by changing all this kind of asserts:

for i in range(10):
....:    print i,
....:    
....: 

to

for i in range(10):
....:    print i,
....:

And, so, not needing trailling spaces anymore.
But this was just to place another road on this issue.
Nice job anyway.

Thomas Kluyver
Collaborator

Without digging into the doctest machinery, I don't know if that would work - the doctests are supposed to represent exactly the pattern of interaction in an IPython session, which does leave trailing spaces there.

gabriellima

Yeah, I was also thinking about it, and if it's the exact intention of test to ensure that it can have:

..something..:
...:
...:

kind of code.
Anyway, nice thing to be done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Showing 1 unique commit by 1 author.

Oct 04, 2011
Bernardo B. Marques remove all trailling spaces 34c1043
Something went wrong with that request. Please try again.