CLN: trying isort-dev #32489
CLN: trying isort-dev #32489
Conversation
| from pandas._libs.tslibs.conversion cimport convert_to_tsobject | ||
| from pandas._libs.tslibs.nattype cimport NPY_NAT, c_NaT as NaT | ||
| from pandas._libs.tslibs.timedeltas cimport convert_to_timedelta64 | ||
| from pandas._libs.tslibs.timezones cimport get_timezone, tz_compare |
There was a problem hiding this comment.
putting tslibs imports before non-tslibs imports (within cython files) is intentional, can we update config to preserve this?
There was a problem hiding this comment.
There is an option to have groups in isort and then order them, so if we have two modules foo and bar, and we want bar first, we can do:
(in setup.cfg)
[isort]
known_foo = pandas.foo
known_bar = pandas.bar
sections = bar,foo
we can achieve the desired behavior but it will be applied across all files (py, pyx, pxi).
Edit:
I have opened an issue at isort asking that question.
ref: PyCQA/isort#1162
| PyDateTime_IMPORT | ||
|
|
||
| import numpy as np | ||
|
|
There was a problem hiding this comment.
if there is a viable way to avoid this newline (and the one on 45), id like to maintain the pattern of keeping all the numpy imports together
There was a problem hiding this comment.
I couldn't achieve that unfortunately, my guess is that isort is separates import and cimport even if it's importing the same library/module
|
-0.5 on the cython edits here. I'm fine with the non-cython edits. Detecting unused imports in cython would be nice. |
| @@ -438,7 +438,7 @@ def __init__( | |||
| elif isinstance(data, dict): | |||
| mgr = init_dict(data, index, columns, dtype=dtype) | |||
| elif isinstance(data, ma.MaskedArray): | |||
| import numpy.ma.mrecords as mrecords | |||
| from numpy.ma import mrecords | |||
There was a problem hiding this comment.
Hmm actually now that I think about it is this an isort thing or something you are suggesting as a standard?
These aren't entirely equivalent and this change is more prone to circular imports. The former is actually suggest by Guido as the way to go so I don't think this is worth changing. (can't find link atm, but its somewhere in the Python docs)
There was a problem hiding this comment.
This is in fact an isort thing (in the dev version), If you can find the link I think we should open an issue about this (to isort).
The first commit is the changes made by isort, the second commit was done manually to fix the pattern of:
from foo.bar import baz as bazThere was a problem hiding this comment.
import numpy.ma doesnt import mrecords into the np.ma namespace. Is this pattern ever liable to trip us up with either form of import?
There was a problem hiding this comment.
@MomIsBestFriend here is what I was describing:
Guido van Rossum recommends avoiding all uses of from import ..., and placing all code inside functions.
So while this is inside a function, I think still not worth changing to a usage that is discouraged by Guido himself
There was a problem hiding this comment.
Thank you @WillAyd for sending the link!
I have opened an issue about this discussion at isort.
xref PyCQA/isort#1164
There was a problem hiding this comment.
Regardless of the from x import y conversation, these usages are not needed since you are not renaming:
import numpy.na.mrecords is the same as import numpy.na.mrecords as mrecords (will probably help you at least workaround this decision by isort team).
|
Closing this as this meant to be a pilot, to see if we will have problems with |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffTrying
isort5.0.0 which is still under development.