PEP8 fixes on dates.py #1398
PEP8 fixes on dates.py #1398
Conversation
|
+1 |
|
|
||
| dt = datetime.datetime(1011, 10, 9, 13, 44, 22, 101010, tzinfo=tz) | ||
| x = date2num(dt) | ||
| _close_to_dt(dt, num2date(x, tz)) |
There was a problem hiding this comment.
Is it worth turning this into a regression test?
There was a problem hiding this comment.
I don't know what pytz is. Any insight on this ?
|
👍 - congratulations @NelleV, I think this is the last PEP8 PR remaining. I take my hat off to you for doing this, you've touched more lines of mpl than I dare imagine, and in doing so have really brought much of the codebase upto PEP8 whitespace standards. We still have a long way to go to be truly "PEP8" compliant (getters and setters being the biggest one), but you've put us on a path which, IMHO, is as important as all the new features of mpl combined - a consistent and more readable codebase. @NelleV - I'm sure there are still plenty of PEP8 whitespace issues, but I wonder if you have considered implementing a PEP8 whitespace test? I have done such a thing in another project and think it is really valuable (but it does have its costs) if your interested: https://github.com/SciTools/cartopy/blob/master/lib/cartopy/tests/test_pep8.py. Thanks again, Phil |
|
Note: There are some |
|
Thanks @dmcdougall - see the shiny new test I linked to in cartopy (which runs the pep8 tool as part of a test). |
|
Ah yes, for some reason I missed the link. Thanks. |
|
@pelson I'm working on moving the code I deleted from the file to unit tests. I'd suggest running flake8 instead of pep8 in your tests: it runs pyflakes in addition to pep8, which allowed me to spot some bugs in the codebase without running it. I can add this to the matplotlib test suites, or it could be run automatically as part as the travis pipeline, as well as test coverage (I don't know travis very well, so I'm not 100% sure this is possible). |
PEP8 fixes on the module dates.
Thanks,
N