These are chat archives for astropy/astropy

5th
Feb 2016
Brigitta Sipocz
@bsipocz
Feb 05 2016 16:32
@astrofrog - Are you here?
Thomas Robitaille
@astrofrog
Feb 05 2016 16:43
@bsipocz - yes, though I have to leave in 10 minutes - but free until the
Brigitta Sipocz
@bsipocz
Feb 05 2016 16:43
I'm thinking of opening a massive PEP8 PR for fixing the uncontroversial errors and warnings. Looking at the statistics it seems that maybe some of them can be included in the setup.cfg, too (e.g. I'm thinking about some of the whitespace and syntax ones below). However it's very probable I oversaw something and you prefer not to extend the required ones too much.
Also, what do you think about the timing of this? Should I do it after the gsoc application rush is over, to avoid unnecessary conflicts?
77      E201 whitespace after '['
7       E202 whitespace before ')'
120     E203 whitespace before ':'
127     E221 multiple spaces before operator
20      E222 multiple spaces after operator
1071    E231 missing whitespace after ','
199     E241 multiple spaces after ','
7       E711 comparison to None should be 'if cond is None:'
57      E712 comparison to True should be 'if cond is True:' or 'if cond:'
16      E713 test for membership should be 'not in'
7       E714 test for object identity should be 'is not'
6       E721 do not compare types, use 'isinstance()'
needless to say that it's not important, I'm just asking
Thomas Robitaille
@astrofrog
Feb 05 2016 16:47
@bsipocz - I personally feel that it makes sense to introduce these slowly over time. For example I think E711 and E712 should be included
but in general I'm not 100% sure about going for the more detailed ones - certainly I think now is not a good time with GSoC as you mentioned, we want to avoid too much rebasing
I wonder if there is a way that we could basically avoid PEP8 regressions so we are always getting better over time
like coverage
but without having to do a massive PR to change everything, which will conflict with all other open PRs
Maybe you could start off with adding ones like E711 to E721?
should be low impact
for E711 and E721 they are low impact so I think it's fine to do it anytime
for the others, I think it makes sense to raise it on the mailing list
Brigitta Sipocz
@bsipocz
Feb 05 2016 16:51
OK. I just thought of this as a braindead cleanup task I could do when not feeling 100% right.
Thomas Robitaille
@astrofrog
Feb 05 2016 16:53
Personally I do think it would be worth fixing, don't get me wrong, but just trying to think about how to minimize impact
it's worthwhile, but not worth causing 50 rebases for example
Brigitta Sipocz
@bsipocz
Feb 05 2016 16:53
yes, I had the same feeling. I would rather avoid the latter :)
Thomas Robitaille
@astrofrog
Feb 05 2016 16:53
btw, you know about autopep8, right? (You can fix specific errors with it)
Brigitta Sipocz
@bsipocz
Feb 05 2016 16:54
sure. I didn't think fixing the 10k pep8 by hand ;)
Thomas Robitaille
@astrofrog
Feb 05 2016 16:54
you could also do fixes to modules where there aren't (m)any PRs open
hehe
for example astropy.vo doesn't have much activity these days
so probably fine to fix
not sure if there are any astropy.visualization PRs open
so for those you can be more aggressive (but can't add to setup.cfg)
also, I think we discussed before that we are not going to worry about the line length limit
Brigitta Sipocz
@bsipocz
Feb 05 2016 16:55
OK. I'll put in the E7** soon, and leave the rest for march/april
Thomas Robitaille
@astrofrog
Feb 05 2016 16:55
Perfect, thanks!
I have to run, but thanks for working on this!
Brigitta Sipocz
@bsipocz
Feb 05 2016 16:57
Yep, line and whitespace around arith operator shouldn't be put in setup.cfg
Sure, thanks for the input and have a nice weekend
Thomas Robitaille
@astrofrog
Feb 05 2016 16:57
you too!