What's the consensus about multiples-of-four indentation?

101 views
Skip to first unread message

Joachim Durchholz

unread,
Jan 2, 2015, 3:39:09 AM1/2/15
to sy...@googlegroups.com
Just a quick survey to find out what's the consensus about what (if
anything) should be done for multiples-of-four indentation.

(Full disclosure: I started to draft this mail during discussion with
Sergey, before he locked me out of the PR discussion. I'm finishing it
because I believe the results of the survey are going to be helpful for
whatever anybody is doing about indentation.)


1) Python has a PEP8 guideline that indents should be a multiple of four.
We do not currently enforce this in test_code_quality.
The consequence is that our codebase currently has two dozen spots where
you can find code that is definitely misindented, plus additional spots
with room for judgement calls.

Would it be desirable to have the guideline enforced?
(Remaining questions are irrelevant if the answer is "no".)


2) To what kinds of lines do we want the rule enforced?
If the answer to (1) was "yes", then it must be applied to executable,
non-continuation lines (the above-mentioned two dozen misindented lines).

2a) Should the rule be applied to continuation lines?
Typical case as found by me in the codebase:
a = self.fubar_the_worg(A, x) \
+ make_me_happy(x, y, z)
And this other category of continuation lines:
klaatu = self.barada_nikto(x, y, z,
a, b, c)
klaatu = self.barada_nikto(x, y, z,
a, b, c)
klaatu = self.barada_nikto(
x, y, z, a, b, c)

2b) Should the rule be applied to comment lines?
Typical situations in our code base:
blah() # Here we need to blah()
# to avoid frobnication
blub(a, x, d, w + x + y + z)
# w2 + x + y + z


3) If we want to handle continuation lines differently than normal
lines, the coding goes beyond what's easily doable in test_code_quality.
pep8.py already has this kind of code, though it is not yet 100% clear
whether it can be made to test everything exactly the way we want it tested.

If pep8.py can be made to do the tests in the way we want them done,
should we use it?


4) How do we want to handle external dependencies in general?
E.g. pep8.py is currently exhibiting a bug that makes it unsuitable for
our purposes (this is likely to get fixed in the near future). This is a
nice reminder that we need to nail any external dependencies down to a
specific version, and upgrade them only after verifying that the new
version does not exhibit any problems. We might even want to postpone
such an upgrade because we want to fix some deficit in SymPy's code first.

4a) Should we simply put a copy of the external dependency into SymPy?
This is what we've been doing for mpmath.
The downside is that we need to flag that code so that people know it's
external, and don't try to fix issues with it.
It might also conflict with other software people might want to run in
the same virtualenv as SymPy, i.e. people will be unable to upgrade or
downgrade to another version (which, of course, still risks breaking
SymPy, but having a dubious option might be better for the user than
having no option at all).
On the plus side, it's hassle-free for users and contributors.

4b) Should we require users to pip install the dependencies?
This makes the learning curve slightly steeper.
It also requires that the user be online when he finds he needs the
dependency, giving a frustrating early experience (which is bad but
fortunately won't happen to many).

4c) Installing dependencies through pip is not a hassle if SymPy itself
is installable through pip. Right now, SymPy is not prepared for that.
Should be go that route?

4d) Should the code that needs the dependency (sympy for mpmath,
test_code_quality for pep8.py) run pip install?
The downside is that being online is going to be reported as a
potentially confusing error message from the bowels of the system; also,
that it is more likely to happen for some dependencies (e.g. the missing
pep8.py could go unnoticed for quite a while until the user does his
first bin/test run).
The plus side is that for the usual case (all required web sites are up
and reachable), it is one installation step less that users need to do
before they work with (or on) SymPy.
Note that pip install can be made quite painless by providing a
dependency file. If users are supposed to install SymPy though pip
anyway, and old versions of a software package cannot ever vanish from
their download URLs, then this is easiest. (I think that the latter does
not hold in general, unfortunately.)


Sorry for presenting you with yet another wall of text, but I didn't
find a way to make this shorter.

Regards,
Jo

Sergey Kirpichev

unread,
Jan 2, 2015, 9:17:23 AM1/2/15
to sy...@googlegroups.com
On Friday, January 2, 2015 11:39:09 AM UTC+3, Joachim Durchholz wrote:
2a) Should the rule be applied to continuation lines?
Typical case as found by me in the codebase:
     a = self.fubar_the_worg(A, x) \
       + make_me_happy(x, y, z)

No errors here.

And this other category of continuation lines:
     klaatu = self.barada_nikto(x, y, z,
                                a, b, c)
     klaatu = self.barada_nikto(x, y, z,
         a, b, c)
     klaatu = self.barada_nikto(
         x, y, z, a, b, c)

FYI: The only error here is the second example.
 
2b) Should the rule be applied to comment lines?
Typical situations in our code base:
     blah() # Here we need to blah()
            # to avoid frobnication
     blub(a, x, d, w + x + y + z)
                 # w2 + x + y + z

E116.  Please note, that this error is not enabled in my
PR
(and also E261 on the first line, that's
disabled too)
.

If pep8.py can be made to do the tests in the way we want them done,
should we use it?

Sure.  SymPy is about symbolic computer algebra, stylistic validators
out of the subject.

4) How do we want to handle external dependencies in general?
E.g. pep8.py is currently exhibiting a bug that makes it unsuitable for
our purposes (this is likely to get fixed in the near future).

Just like we handle them now (or do this other projects, whatever).  I.e. - we should
specify version requirements.
 
4a) Should we simply put a copy of the external dependency into SymPy?

We already have discussion about this while unbundling mpmath. The end
of it, I think - that this is bad practice in general (though we can have
reasons to do this).
 
4c) Installing dependencies through pip is not a hassle if SymPy itself
is installable through pip. Right now, SymPy is not prepared for that.

What's wrong with this?

$ pip install sympy
Downloading/unpacking sympy
  Downloading sympy-0.7.6.tar.gz (6.4Mb): 6.4Mb downloaded
  Running setup.py egg_info for package sympy
   
Installing collected packages: sympy
  Running setup.py install for sympy
    changing mode of build/scripts-2.7/isympy from 640 to 755
   
    changing mode of /home/sk/.virtualenvs/sympy1/bin/isympy to 755
Successfully installed sympy
Cleaning up...
$ isympy --version
0.7.6

Please do trivial checks for your claims.
 
4d) Should the code that needs the dependency (sympy for mpmath,
test_code_quality for pep8.py) run pip install?

Not in the case of test_code_quality.py.  test_*.py - for testing,
not for installation of dependencies.

Joachim Durchholz

unread,
Jan 2, 2015, 6:17:15 PM1/2/15
to sy...@googlegroups.com
Am 02.01.2015 um 15:17 schrieb Sergey Kirpichev:
> On Friday, January 2, 2015 11:39:09 AM UTC+3, Joachim Durchholz wrote:
>>
>> 2a) Should the rule be applied to continuation lines?
>> Typical case as found by me in the codebase:
>> a = self.fubar_the_worg(A, x) \
>> + make_me_happy(x, y, z)
>
> No errors here.

Okay, so your vote is "backslash continuation lines do not need to be
indented by multiples of four".

>> klaatu = self.barada_nikto(x, y, z,
>> a, b, c)
>> klaatu = self.barada_nikto(x, y, z,
>> a, b, c)
>> klaatu = self.barada_nikto(
>> x, y, z, a, b, c)
>
> FYI: The only error here is the second example.

It's a bit unfortunate that these indents happened to be multiples of
four, so they're all non-erroneous no matter what...

So, what's your stance on the following:

# indent by 2
klaatu = self.barada_nikto(x, y, z,
a, b, c)
klaatu = self.barada_nikto(
x, y, z, a, b, c)
# indent by 7, matches the first operator on the line
klaatu = self.barada_nikto(x, y, z,
a, b, c)
klaatu = self.barada_nikto(
x, y, z, a, b, c)

>> 2b) Should the rule be applied to comment lines?
>> Typical situations in our code base:
>> blah() # Here we need to blah()
>> # to avoid frobnication
>> blub(a, x, d, w + x + y + z)
>> # w2 + x + y + z
>
> E116. Please note, that this error is not enabled in my
> PR (and also E261 on the first line, that's
> disabled too).

Does this mean that you think it should be flagged, or not?

>> If pep8.py can be made to do the tests in the way we want them done,
>> should we use it?
>
> Sure. SymPy is about symbolic computer algebra, stylistic validators
> out of the subject.

Vote noted.

> 4) How do we want to handle external dependencies in general?
>> E.g. pep8.py is currently exhibiting a bug that makes it unsuitable for
>> our purposes (this is likely to get fixed in the near future).
>
> Just like we handle them now (or do this other projects, whatever). I.e. -
> we should specify version requirements.

Vote noted.

>> 4c) Installing dependencies through pip is not a hassle if SymPy itself
>> is installable through pip. Right now, SymPy is not prepared for that.
>
> What's wrong with this?
>
> $ pip install sympy

It's undocumented, i.e. it might not work anymore with the next SymPy
version.

> Please do trivial checks for your claims.

(Please do not accuse based on an interpretation that wasn't intended by
the other side.)

>> 4d) Should the code that needs the dependency (sympy for mpmath,
>> test_code_quality for pep8.py) run pip install?
>
> Not in the case of test_code_quality.py. test_*.py - for testing,
> not for installation of dependencies.

Vote noted.
Out of curiosity: What problems do you see with that?

Sergey B Kirpichev

unread,
Jan 2, 2015, 7:09:54 PM1/2/15
to sy...@googlegroups.com
On Sat, Jan 03, 2015 at 12:17:12AM +0100, Joachim Durchholz wrote:
> >No errors here.
>
> Okay, so your vote

No. It's not my vote, it's PEP 8.

> >> klaatu = self.barada_nikto(x, y, z,
> >> a, b, c)
> >> klaatu = self.barada_nikto(x, y, z,
> >> a, b, c)
> >> klaatu = self.barada_nikto(
> >> x, y, z, a, b, c)
> >
> >FYI: The only error here is the second example.
>
> It's a bit unfortunate that these indents happened to be multiples
> of four, so they're all non-erroneous no matter what...

Not all.

> So, what's your stance on the following:

Just run pep8, please. Or read PEP 8.

In short, no - these indents sometimes may be indented
to other than 4 spaces.

> Does this mean that you think it should be flagged, or not?

It's just out of discussion for my pr.

Clearly, that we have to decide first - should we use pep8 testing
at all, instead of the current code quality tests. If we decide on
this positively - next we could enable specific error codes one by one.

> >>4c) Installing dependencies through pip is not a hassle if SymPy itself
> >>is installable through pip. Right now, SymPy is not prepared for that.
> >
> >What's wrong with this?
> >
> >$ pip install sympy
>
> It's undocumented, i.e. it might not work anymore with the next
> SymPy version.

I assure you, next release will be available on PyPI
as well. And you can read installation instructions right
there (see "Get Packages"): https://pypi.python.org/pypi

Joachim Durchholz

unread,
Jan 3, 2015, 7:09:01 AM1/3/15
to sy...@googlegroups.com
>>>> a = self.fubar_the_worg(A, x) \
>>>> + make_me_happy(x, y, z)

(Please do not cut the example, Sergey, I had to go back and look up the
exact situation.)

>>> No errors here.
>>
>> Okay, so your vote
>
> No. It's not my vote, it's PEP 8.

PEP8 is ambiguous about that. It says that multiples-of-four is
"optional" for hanging indents, but it does not say who's choosing the
option: The project or the individual coder. I.e. I'm taking this as a
vote for "the coder chooses" (which is in line with what I know about
other sympyists' stances).

>>>> klaatu = self.barada_nikto(x, y, z,
>>>> a, b, c)
>>>> klaatu = self.barada_nikto(x, y, z,
>>>> a, b, c)
>>>> klaatu = self.barada_nikto(
>>>> x, y, z, a, b, c)
>>>
>>> FYI: The only error here is the second example.
>>
>> It's a bit unfortunate that these indents happened to be multiples
>> of four, so they're all non-erroneous no matter what...
>
> Not all.

Please explain.
My reading of PEP8 is that it's always okay to indent by multiples of four.

>> So, what's your stance on the following:
>
> Just run pep8, please. Or read PEP 8.

I want to know what conclusions you (and others) draw from PEP8.
I know my own interpretation well enough thank you very much, but that's
not going to get codified (and with good reason, my interpretations
aren't necessarily more accurate than anybody else's).

> In short, no - these indents sometimes may be indented
> to other than 4 spaces.

Okay, vote noted.

>> Does this mean that you think it should be flagged, or not?
>
> It's just out of discussion for my pr.

Which is related to this survey, but off topic. This isn't for
discussing your PR, it's for laying foundations that you can use for
your PR.
(Besides, you closed that PR - what's the point in still discussing it?)

> Clearly, that we have to decide first - should we use pep8 testing
> at all, instead of the current code quality tests.

This very survey is based on the assumption that we should first get a
clear picture of what we want, then select the tools.
One can reasonably disagree about that, but that's off-topic for this
thread. I added the (4) questions just to have the pep8.py case covered
*in case we need to use it*. If the consensus were that even
continuation lines should be indented by multiples of four, we wouldn't
need pep8.py. I do not expect that to happen, but I do not want to rule
it out just because of my expectations.

>>>> 4c) Installing dependencies through pip is not a hassle if SymPy itself
>>>> is installable through pip. Right now, SymPy is not prepared for that.
>>>
>>> What's wrong with this?
>>>
>>> $ pip install sympy
>>
>> It's undocumented, i.e. it might not work anymore with the next
>> SymPy version.
>
> I assure you, next release will be available on PyPI
> as well.

Has this been discussed and decided?

Sergey B Kirpichev

unread,
Jan 3, 2015, 7:25:44 AM1/3/15
to sy...@googlegroups.com
On Sat, Jan 03, 2015 at 01:08:57PM +0100, Joachim Durchholz wrote:
> but it does not say who's choosing
> the option: The project or the individual coder.

Sure, both. The project should allow this to be possible for
developers.

> Besides, you closed that PR

I could thank you for this.

Joachim Durchholz

unread,
Jan 3, 2015, 7:41:41 AM1/3/15
to sy...@googlegroups.com, asme...@gmail.com, srjogl...@gmail.com, smichr@gmail.com >> smichr, cris...@umn.edu, kundanku...@gmail.com, mrs...@gmail.com, macd...@maths.ox.ac.uk, rao...@gmail.com, franz....@gmail.com
Sergey's feedback is certainly relevant, but to date, it's just him and me.
This *is* policymaking stuff, and I'd feel rather uncomfortable if we
just decided something and charged ahead, just to hear later that people
are unhappy about it.

(CC'ing this to whose who were among the top 20 line-adding contributors
both in 2013 and 2014. My apologies if you did not want to receive this.)

Joachim Durchholz

unread,
Jan 3, 2015, 7:44:16 AM1/3/15
to sy...@googlegroups.com
Am 03.01.2015 um 13:25 schrieb Sergey B Kirpichev:
> On Sat, Jan 03, 2015 at 01:08:57PM +0100, Joachim Durchholz wrote:
>> but it does not say who's choosing
>> the option: The project or the individual coder.
>
> Sure, both. The project should allow this to be possible for
> developers.

So not the project's option.
That's okay, it's a vote.

>> Besides, you closed that PR
>
> I could thank you for this.

Taking that to private mail.

Joachim Durchholz

unread,
Jan 3, 2015, 8:06:00 AM1/3/15
to sy...@googlegroups.com
Am 03.01.2015 um 13:25 schrieb Sergey B Kirpichev:
You should thank yourself.

I did make decisions, yes.
But not this one. You could have decided otherwise; for example, you
could have worked on finding out what's unclear, fixing what needed
fixing in the SymPy project's opinion (which isn't necessarily the same
as mine), and moved forward.

Closing the PR was just the fastest way out of a situation you found
unpleasant.
It was not the best way to move the issue forward though.

Joachim Durchholz

unread,
Jan 3, 2015, 8:43:01 AM1/3/15
to sy...@googlegroups.com
Argh, sorry, this should have gone to private mail.

Chris Smith

unread,
Jan 7, 2015, 12:14:59 PM1/7/15
to sy...@googlegroups.com
I can't think of a compelling reason not to always use multiples of 4. WRT the continuation of parameters, one may just move all parameters down to the next indent as

def foo(
        a, b, c,
        d, e, f):
    # first line of func
    ...


Joachim Durchholz

unread,
Jan 7, 2015, 1:24:31 PM1/7/15
to sy...@googlegroups.com
Am 07.01.2015 um 18:14 schrieb Chris Smith:
> I can't think of a compelling reason not to always use multiples of 4.

"Don't forbid stuff unless necessary" would be one :-)

> WRT
> the continuation of parameters, one may just move all parameters down to
> the next indent as
>
> def foo(
> a, b, c,
> d, e, f):

That's an indent of nine spaces. I suppose that's unintentional, I guess
you meant to say

def x(
a, b, c
d, e, f):
# first line of x

This would work, and allow a simple "every indent no matter what should
be a multiple of four" rule, fit for check_code_quality.

However, it would probably annoy some people, because it goes beyond the
PEP8 consensus, which allow almost arbitrary indents on this kind of
continuation line (technically, an indent of 1 would be permissible).

The question here is: Is an easier check worth the additional annoyance?
Is the better code conformity (which makes it easier to scan quickly)
worth it?
How bad is the annoyance? It means that if you run bin/test before
issuing the pull request, you may have to go over your code and fix a
dozen or more lines. How would people feel about that? Would they
consider that bondage&discipline, or would they accept that on the basis
of "ah well whatever"?

I have no universal answers for these questions; it's essentially all
judgement calls.
I have one vague and not very definitive answer: large projects
generally tend to have even stricter style guidelines; the most
well-known being Linux, and the GNU software collection.

Not having definitive answers is the reason why I'm asking for votes :-)

Ondřej Čertík

unread,
Jan 7, 2015, 1:38:14 PM1/7/15
to sympy
On Wed, Jan 7, 2015 at 11:24 AM, Joachim Durchholz <j...@durchholz.org> wrote:
> Am 07.01.2015 um 18:14 schrieb Chris Smith:
>>
>> I can't think of a compelling reason not to always use multiples of 4.
>
>
> "Don't forbid stuff unless necessary" would be one :-)
>
>> WRT
>>
>> the continuation of parameters, one may just move all parameters down to
>> the next indent as
>>
>> def foo(
>> a, b, c,
>> d, e, f):
>
>
> That's an indent of nine spaces. I suppose that's unintentional, I guess you
> meant to say
>
> def x(
> a, b, c
> d, e, f):
> # first line of x
>
> This would work, and allow a simple "every indent no matter what should be a
> multiple of four" rule, fit for check_code_quality.
>
> However, it would probably annoy some people, because it goes beyond the
> PEP8 consensus, which allow almost arbitrary indents on this kind of
> continuation line (technically, an indent of 1 would be permissible).
>

Here are my opinions:

> The question here is: Is an easier check worth the additional annoyance?

No.

> Is the better code conformity (which makes it easier to scan quickly) worth
> it?

No.

> How bad is the annoyance?

Bad. We should only check the very bad stuff like implicit imports or
trailing whitespace.

> It means that if you run bin/test before issuing
> the pull request, you may have to go over your code and fix a dozen or more
> lines. How would people feel about that?

I would be annoyed.

> Would they consider that
> bondage&discipline, or would they accept that on the basis of "ah well
> whatever"?

If the code quality is low, the pull request reviewer should bring
this up in each particular case. We should use 4 spaces for
indentation, but we should allow exceptions and multiline statements
seem like one.

My suggestion would be to move on to do some more productive work than
worry about this.

Ondrej

Joachim Durchholz

unread,
Jan 7, 2015, 2:08:54 PM1/7/15
to sy...@googlegroups.com
Am 07.01.2015 um 19:38 schrieb Ondřej Čertík:
> My suggestion would be to move on to do some more productive work than
> worry about this.

I'm more after finding out what the actual consensus is so that I can
configure my checking tools properly.
Also, I'd like to find out what checks, if any, should be added to
check_code_quality.

Sergey B Kirpichev

unread,
Jan 7, 2015, 3:07:52 PM1/7/15
to sy...@googlegroups.com
On Wed, Jan 07, 2015 at 11:38:11AM -0700, Ondřej Čertík wrote:
> If the code quality is low, the pull request reviewer should bring
> this up in each particular case. We should use 4 spaces for
> indentation, but we should allow exceptions and multiline statements
> seem like one.

There are real problems, that #8538 was trying to solve.

1) during reviews, stylistic comments sounds like noise for
me and I would like to avoid this at all cost. Make all
this (or part of) to be subject of the reqular pr testing
seems to be a wise idea. Despite the fact that sometimes
the tool can blame developers in some questionable cases.

2) Stylistic commits - also is a noise in the git blame. Try
to use one on the sympy/core and you can easily hit on
some "PEP XYZ" commit during the review of almost every
line history.

3) Style validation (i.e. test_code_quality.py) goes out of
the project. Other people know this better, lets do
symbolic math instead.

Aaron Meurer

unread,
Jan 7, 2015, 3:09:10 PM1/7/15
to sy...@googlegroups.com
I agree with everything Ondrej said.

Aaron Meurer

--
You received this message because you are subscribed to the Google Groups "sympy" group.
To unsubscribe from this group and stop receiving emails from it, send an email to sympy+un...@googlegroups.com.
To post to this group, send email to sy...@googlegroups.com.
Visit this group at http://groups.google.com/group/sympy.
To view this discussion on the web visit https://groups.google.com/d/msgid/sympy/CADDwiVDvU9ZdZs3OmBp3JzL0kHKv4oZ1Ng7buTH2Q0EnZM18BQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

Joachim Durchholz

unread,
Jan 7, 2015, 3:38:05 PM1/7/15
to sy...@googlegroups.com
Am 07.01.2015 um 21:08 schrieb Aaron Meurer:
> I agree with everything Ondrej said.

Thanks, I think that establishes the consenus:

Code lines must be indented by multiples of four spaces, except for
continuation and comment lines where we do not impose any restrictions
(beyond what the Python compiler requires).

Is that correct?
If so, I'll put that in the doc pages.
Reply all
Reply to author
Forward
0 new messages