Repository navigation
Fix all pep8 lint - #616
Conversation
|
I'd like to wait for #614 before merging this. |
|
Can you rebase the PR to have the new travis config? |
8a6d1e9 to
489ecd8
Compare
|
Big fan of pep8'ing things. The less mental space assigned to formatting -- whatever defaults are chosen -- the better in my opinion. That said, since this is probably as good of a time as any to ask, why 120 vs 79 for column limit? I personally go back and forth, but often wonder if the horizontal space gained is worth the exception from community norm. |
|
I could never understand why is it 79, not 80... |
I like the GitHub rule: GitHub has a max line length of ~120 when displaying diffs. Happy to change to whichever value the @pytorch folks like. |
|
let's not discuss 120 vs 79 anymore :D |
|
@lukeyeager I've pushed a fix for a flaky Sqrt test and it's conflicting with your changes now. Can you reabse them once more please? |
Here's the command I used to invoke autopep8 (in parallel!):
git ls-files | grep '\.py$' | xargs -n1 -P`nproc` autopep8 -i
Several rules are ignored in setup.cfg. The goal is to let autopep8
handle everything which it can handle safely, and to disable any rules
which are tricky or controversial to address. We may want to come back
and re-enable some of these rules later, but I'm trying to make this
patch as safe as possible.
Also configures flake8 to match pep8's behavior.
Also configures TravisCI to check the whole project for lint.
489ecd8 to
c115b42
Compare
|
Rebased. I discarded my conflicting changes, took yours, then ran autopep8 on top of them again. |
|
@pytorchbot add to whitelist |
|
Thanks! |
|
@lukeyeager Does the config have to be in |
|
I know, it bugs me too. Here are the other options:
|
|
Hmmm ok, we're not using |
|
@lukeyeager, would putting setup.cfg into tools directory and passing --config=tools to autopep8 be an option? |
|
Good point, that's an option too. The advantage of keeping the config file in a standard location is that you can call |
|
It's not that bad, let's keep it in the root for now. We'll have to start using |
|
If |
Enabling tensor index from random_split
…devel Merged master 21 01 25 devel
7 participants
Close #3
This is a follow-up to #575. This is that terrifying diff I was talking about.
To reduce the terror, I've split this up into several commits: