CLN: PEP8 cleanup #5038
CLN: PEP8 cleanup #5038
Conversation
|
I don't want to reflow docstrings and comments like that. Docstrings are long enough that I think it would impair their readability. |
| Handles Python2/3 compatibility transparently.""" | ||
| # side note - this could be made into a metaclass if more than one object nees | ||
|
|
||
| # side note - this could be made into a metaclass if more than one object |
There was a problem hiding this comment.
you can just delete this comment.
|
and @alefnula this only can be done when nothing major is going on.....so will have to wait on this |
| @@ -1832,7 +1850,8 @@ def _astype_nansafe(arr, dtype, copy=True): | |||
| return arr.view(dtype) | |||
| elif dtype != _NS_DTYPE: | |||
| raise TypeError( | |||
| "cannot astype a datetimelike from [%s] to [%s]" % (arr.dtype, dtype)) | |||
| "cannot astype a datetimelike from [%s] to [%s]" % (arr.dtype, | |||
There was a problem hiding this comment.
can you fix this so it looks better? (i.e., move the string back up to the opening paren of the TypeError and then reflow it.
|
@jreback OK. Then I'll close this pull request since it apparently won't be merged. If you decide, at any time, to do this cleanup I'm willing to volunteer... |
|
Separate note - You should separate fixing imports from fixing formatting (even though they are related) because imports can be a bit trickier in certain places (and have a greater probability for causing issues when rebasing). Also, do people think we should pep8 the test cases? It's a bit of a bummer to do it because they require huge changes, often end up looking ugly, and you have to work a bit harder to figure out when the tests were inserted. |
|
@alefnula we'll do it eventually though. it's just hard to deal with PEP8 changes when we have big PRs brewing that touch many parts of pandas. |
|
@alefnula if you wanted to put together a "remove extraneous imports" PR, that would be very welcome. (I'd suggest waiting a few days for the dust to settle after this PR I'm hoping to get in today). |
|
@jtratner I understand the reasons completely :) That's why I said: at some point in time... |
|
@jtratner Fixed, rebased, squashed... |
|
nope.....I am ok with this....just wanted to get your change in first (and mine!) |
|
@jtratner @jreback Just discard this pull request and continue with your regular work :) When the time is right I'll do the pep8 fixing again. In this PR I just did it for a few module, more as a proposal, so it's not a big deal... I will do it again for the whole library when you think the timing is better. |
|
@alefnula you should do it in pieces, so it's easier to review everything. I'd start with non-tests (maybe do computation, then everything non-core, then core), and then after the dust has settled on that, start on tests. (if you change the test cases at the same time, there's the potential for something to break without realizing it) |
|
@jtratner OK, I'll do it that way then. Just trigger me when you think that the time is right for these sort of changes. |
|
@alefnula btw - you should checkout pycharm - it has a "cleanup imports" feature that could make it simple to fix all the imports (it does fail in the decorators file though) |
|
@jtratner I'm using PyDev :) It has the same features :) And better font aliasing :P |
|
@alefnula interesting - I pretty much only use vim. I like PyCharm for some of its checkers though. Haven't tried PyDev. |
|
@alefnula nows your chance to do this. Please do two separate commits, one for pep8 and one for import cleanup. About to embark on some stuff that's going to touch a few places so I'd like to get it over with :P |
|
Btw - You can just reuse this PR (force push to the branch) |
|
@jtratner OK :) I'll do it this weekend. |
|
@jtratner Please comment if you don't like the style I used to make to code "pep8 compatible". |
|
I hate to say this, but if you were able to make at least the main library pass flake8, that would be helpful for reducing the noise so it would be easier to lint the codebase on a regular basis. Is there anything that would get 'ugly' if we did that? Can you list the changes from spellcheck? Those are probably most concerning to me (i.e., can't change public method names!) |
|
@jtratner I don't see the any Currently I reformatted the And about the spellcheck - its in the docstrings not in any public API... I haven't touched any of the functionality, nor I intend to. What I meant when I said spellcheck is basically docstrings and comments. |
|
@alefnula okay, that's fine. I guess we don't have a settled syntax checker (though there's a lot of pylint directives) so it's not worth focusing on. Thanks for making these fixes! |
|
If the style is OK I'll continue through the rest of the code... |
|
yes looks fine to me |
|
@alefnula how about we pause wherever you are right now, merge that part and then you can work on finishing cleaning up all the tests? |
|
@jtratner I'm OK with whatever you decide. I rebased it. |
|
Okay, please open a second pull request to cover the rest (you can just branch off this PR), that way I can review this set just to make sure nothing looks weird. |
CLN: PEP8 cleanup
|
@alefnula thanks! now on to part2 |
I stated a PEP8 cleanup and did it for a few modules. Changed only the things where this doesn't produce ugly code.
If this is acceptable, I can continue doing this and go through the whole pandas codebase. There are just a few things that I would like to check if this is acceptable:
timedelta2 times for example in the cleanup that I did)...a[i+2:3]this is against PEP8, it should look likea[i + 2:3], but I don't think these things should be changed, the second one is uglier even if it's against PEP8.Also one of the problems with this cleanup is that it changes a lot of things and should be merged as soon as possible :( Because it will soon become unmergable...