[lint] organizes python imports #14260
Conversation
iris-garden
force-pushed
the
lint/just-imports
branch
3 times, most recently
from
1e82307
to
f757324
Compare
iris-garden
force-pushed
the
lint/just-imports
branch
3 times, most recently
from
89179c4
to
b33cc85
Compare
iris-garden
force-pushed
the
lint/just-imports
branch
from
b33cc85
to
bd244e8
Compare
iris-garden
force-pushed
the
lint/just-imports
branch
from
bd244e8
to
a029ad4
Compare
| @@ -1,5 +1,4 @@ | |||
| import warnings | |||
| from ..aiocloud.aiogoogle import * # noqa: F403 | |||
There was a problem hiding this comment.
I think this is not quite right. This package was moved but this line exists to re-export those objects under the old package name. The proper way to do this is probably to put all those imports into __ALL__.
There was a problem hiding this comment.
If doing so is painful, we can probably remove this file at this point (it's been a while), but let's do that as a separate PR.
There was a problem hiding this comment.
updated! i just used the __all__ list from aiocloud/aiogoogle/__init__.py to decide what to include, if that seems good
There was a problem hiding this comment.
I'm hesitant about this because now it needs to be kept in sync with aiocloud.aiogoogle. Why did ruff complain about the star import in the first place? Shouldn't it respect the noqa?
There was a problem hiding this comment.
that.....is a very good point. i reverted this file to the version on main and it does not, in fact, complain; it must have at some point? but honestly i don't remember 😅
| @@ -1881,6 +1890,7 @@ def write_file(n, n_rows=5): | |||
| out.write('\t') | |||
| out.write(str(i % 2 == 0)) | |||
|
|
|||
| widths = [1 << k for k in range(8, 14)] | |||
There was a problem hiding this comment.
huh, where's this from?
There was a problem hiding this comment.
this is from dan's PR for linting the tests, we had talked about it at standup at some point; ruff complains that widths is undefined otherwise. we can split this off into a tiny PR and just add an ignore for the error if that works better, just lmk!
There was a problem hiding this comment.
I see, that comes from #7416 and is a historical reference that is unused. Ok we can just keep it as it is in this PR.
| @@ -13,6 +13,31 @@ ignore = [ | |||
| "PLR0913", # Too many arguments to function call | |||
| "PLC1901", # `<VAR> != ''` can be simplified to `<VAR>` as an empty string is falsey | |||
| "PLR2004", # Magic value used in comparison | |||
| "F541", | |||
There was a problem hiding this comment.
Can you leave a comment here that clarifies that we intend to eliminate these added rules as opposed to the existing ones which we indefinitely turned off
iris-garden
force-pushed
the
lint/just-imports
branch
from
a029ad4
to
2a44ef7
Compare
| @@ -1,5 +1,4 @@ | |||
| import warnings | |||
| from ..aiocloud.aiogoogle import * # noqa: F403 | |||
There was a problem hiding this comment.
I'm hesitant about this because now it needs to be kept in sync with aiocloud.aiogoogle. Why did ruff complain about the star import in the first place? Shouldn't it respect the noqa?
iris-garden
force-pushed
the
lint/just-imports
branch
from
2a44ef7
to
25763de
Compare
No description provided.