cleanup inconsistently used imports #19292
Conversation
|
Hello @jbrockmendel! Thanks for submitting the PR.
|
|
I have no problem with doing this, but pls add a lint rule for this, otherwise over time it will just change again. |
pandas/core/generic.py
Outdated
| from pandas.core.common import (_count_not_none, | ||
| _maybe_box_datetimelike, _values_from_object, | ||
| AbstractMethodError, SettingWithCopyError, | ||
| from pandas.core.common import (AbstractMethodError, SettingWithCopyError, |
There was a problem hiding this comment.
this is inconsistent with your change
Codecov Report
@@ Coverage Diff @@
## master #19292 +/- ##
==========================================
- Coverage 91.53% 91.5% -0.03%
==========================================
Files 150 150
Lines 48738 48727 -11
==========================================
- Hits 44611 44588 -23
- Misses 4127 4139 +12
Continue to review full report at Codecov.
|
| # Check for imports from pandas.core.common instead | ||
| # of `import pandas.core.common as com` | ||
| echo "Check for non-standard imports" | ||
| grep -R --include="*.py*" -E "from pandas.core.common import " pandas |
There was a problem hiding this comment.
this rule includes too many things, you want to only accept from pandas.core import common as com. anything else involving pandas.core.common should error
There was a problem hiding this comment.
This is tough to do with grep since there are things like sys.modules['pandas.core.common'], warnings.warn("pandas.core.common.{t} is deprecated, # the pandas.core.common introspectors. Even if I string together a workable grep command, maintaining it will be a PITA. I think the 1-line 98% solution is the way to go on this one.
pandas/core/computation/align.py
Outdated
| @@ -10,7 +10,9 @@ | |||
| import pandas as pd | |||
| from pandas import compat | |||
| from pandas.errors import PerformanceWarning | |||
| from pandas.core.common import flatten | |||
|
|
|||
| import pandas.core.common as com | |||
There was a problem hiding this comment.
I really don't like extra blank lines between imports for the most part, occasionally for readibility
| @@ -8,7 +8,9 @@ | |||
|
|
|||
| import warnings | |||
| import numpy as np | |||
| from pandas.core.common import _values_from_object | |||
|
|
|||
| import pandas.core.common as com | |||
There was a problem hiding this comment.
the one after numpy is ok, but no reason for line 13
| @@ -62,12 +62,6 @@ | |||
| from pandas.core.dtypes.missing import isna, notna | |||
|
|
|||
pandas/tests/util/test_util.py
Outdated
| @@ -8,7 +8,9 @@ | |||
|
|
|||
| import pytest | |||
| from pandas.compat import intern | |||
| from pandas.core.common import _all_none | |||
|
|
|||
There was a problem hiding this comment.
i am not going to comment on all the xtra lines, but pls clean up
There was a problem hiding this comment.
OK. The habit (that I'll try to break) is to add lines between stdlib vs 3rd-party vs pandas-but-not-same-sub-package vs same-subpackage.
|
thanks. generally making things consistent is good. however import format is almost a style thing and can be somewhat arbitrary and is hard to enforce. so limit these type of PR's unless there is really a massive difference or dichotomy across the codebase (which as true for common) |
|
@jbrockmendel how attached are we to importing from Because in there is the argument Lines 11523 to 11527 in 1f6e44a which shadows the import Lines 133 to 136 in 1f6e44a If it's OK, I'd just import that one without an alias and |
|
i dont feel strongly about it |
There are a bunch of modules that will do both
import pandas.core.common as comandfrom pandas.core.common import _whatever, then sprinkled throughout we'll see both_whateverandcom._whatever. This PR tracks down a bunch of those and standardizes oncom._whatever.git diff upstream/master -u -- "*.py" | flake8 --diff