-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Enforce ruff/flake8-pyi rules (PYI) #10359
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
aea49bd
to
aaf75d1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks broadly good to me!
@@ -39,7 +39,6 @@ class Default(Enum): | |||
_default = Default.token | |||
|
|||
# https://stackoverflow.com/questions/74633074/how-to-type-hint-a-generic-numpy-array | |||
_T = TypeVar("_T") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we keep this (private) type even though it is not used?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we stopped using it, unless the link in the line above is relevant?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may be mistaken, but https://stackoverflow.com/questions/74633074/how-to-type-hint-a-generic-numpy-array does not seem relevant to _T
vs. _T_co
. Since both _typing
and _T
are private, I guess it's safe to remove _T
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's defined locally elsewhere:
xarray/xarray/computation/rolling.py
Line 35 in d589df1
_T = TypeVar("_T") |
It looks like the more global definition in xarray/namedarray/_typing.py
is not used any more.
@@ -214,7 +214,7 @@ def copy( | |||
|
|||
# FYI in some cases we don't allow `None`, which this doesn't take account of. | |||
# FYI the `str` is for a size string, e.g. "16MB", supported by dask. | |||
T_ChunkDim: TypeAlias = str | int | Literal["auto"] | None | tuple[int, ...] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the intent was to have int, tuple of int or "auto" — not any string. I hope I'm not wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
check comment above :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was so busy scavenging the reference documentation for a hint that I didn't see the comment 😕
Reverted and added # noqa: PYI051
instead. I could remove Literal["auto"]
since it is covered by str
, but I'd rather keep it for code documentation purposes.
PYI018 Private TypeVar is never used
PYI024 Use `typing.NamedTuple` instead of `collections.namedtuple`
PYI025 Use `from collections.abc import Set as AbstractSet` to avoid confusion with the `set` builtin
PYI032 Prefer `object` to `Any` for the second parameter
PYI034 such methods usually return `self` at runtime
PYI046 Private protocol is never used
PYI051 `Literal["..."]` is redundant in a union with `str`
PYI055 Multiple `type` members in a union. Combine them into one
PYI063 Use PEP 570 syntax for positional-only parameters
By definition `T_Chunks` and `T_Engine` could actually be `None`.
This partially reverts commit 4bc134a.
Rebased. |
nice, thanks @DimitriPapadopoulos ! |
* main: Fix performance regression in interp from pydata#9881 (pydata#10370) html repr: improve style for dropdown sections (pydata#10354) Grouper tweaks. (pydata#10362) Docs: Add links to getting help mermaid diagram (pydata#10324) Enforce ruff/flynt rules (FLY) (pydata#10375) Add missing AbstractWritableDataStore base methods and arguments (pydata#10343) Improve html repr in dark mode (Jupyterlab + Xarray docs) (pydata#10353) Pin Mypy to 1.15 (pydata#10378) use numpy dtype exposed by zarr array instead of metadata.data_type (pydata#10348) Fix doc typo for caption "Interoperability" (pydata#10374) Implement cftime vectorization as discussed in PR pydata#8322 (pydata#8324) Enforce ruff/flake8-pyi rules (PYI) (pydata#10359) Apply assorted ruff/Pylint rules (PL) / Enforce PLE rules (pydata#10366) (fix): pandas extension array repr for int64[pyarrow] (pydata#10317) Enforce ruff/flake8-implicit-str-concat rules (ISC) (pydata#10368) Enforce ruff/refurb rules (FURB) (pydata#10367) Ignore ruff/Pyflakes rule F401 more precisely (pydata#10369) Apply assorted ruff/flake8-simplify rules (SIM) (pydata#10364) Apply assorted ruff/flake8-pytest-style rules (PT) (pydata#10363) Fix "a array" misspelling (pydata#10365)
These rules improve type-checking.
whats-new.rst
api.rst
I believe the sphinx failure in CI is unrelated: