Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Update Ruff and enable F821 in stubs #11771

Merged
merged 1 commit into from
Apr 17, 2024
Merged

Update Ruff and enable F821 in stubs #11771

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .flake8
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
[flake8]
# NQA: Ruff won't warn about redundant `# noqa: Y`
# Y: Flake8 is only used to run flake8-pyi, everything else is in Ruff
# F821: Until https://github.com/astral-sh/ruff/issues/3011 is fixed, we need flake8-pyi's monkeypatching
# F821: Typeshed is a testing ground for flake8-pyi, which monkeypatches F821
select = NQA, Y, F821
# Ignore rules normally excluded by default
extend-ignore = Y090
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ repos:
args: [--fix=lf]
- id: check-case-conflict
- repo: https://github.com/astral-sh/ruff-pre-commit
rev: v0.3.5 # must match requirements-tests.txt
rev: v0.3.7 # must match requirements-tests.txt
hooks:
- id: ruff
# Run this separately because we don't really want
Expand Down
2 changes: 0 additions & 2 deletions pyproject.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,8 +91,6 @@ ignore = [
"E501", # Line too long
"E741", # ambiguous variable name
"F403", # `from . import *` used; unable to detect undefined names
# False positives in stubs
"F821", # Undefined name: https://github.com/astral-sh/ruff/issues/3011
# Stubs can sometimes re-export entire modules.
# Issues with using a star-imported name will be caught by type-checkers.
"F405", # may be undefined, or defined from star imports
Expand Down
2 changes: 1 addition & 1 deletion requirements-tests.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ mypy==1.9.0
pre-commit-hooks==4.5.0 # must match .pre-commit-config.yaml
pyright==1.1.358
pytype==2024.4.11; platform_system != "Windows" and python_version < "3.12"
ruff==0.3.5 # must match .pre-commit-config.yaml
ruff==0.3.7 # must match .pre-commit-config.yaml

# Libraries used by our various scripts.
aiohttp==3.9.3
Expand Down
17 changes: 8 additions & 9 deletions stdlib/typing.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,6 @@ if sys.version_info >= (3, 12):
ContextManager = AbstractContextManager
AsyncContextManager = AbstractAsyncContextManager

# This itself is only available during type checking
def type_check_only(func_or_cls: _F) -> _F: ...
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why were these moved? This is legal in stubs, even if it might be better style to move the function after the definition of _F.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ruff does not currently support generalised forward references in stub files in the same way that mypy and pyright do. We considered implementing that support, but when we looked, we couldn't actually find a reference anywhere in any PEP or typing spec that specified that all forward references should be allowed in stub files. We therefore decided to only support forward references in situations where it could plausibly be useful to have forward references, such as class bases. See astral-sh/ruff#10779 (comment).

This is similar in concept to the limited way flake8-pyi monkeypatches flake8. pyflakes-monkeypatched-by-flake8-pyi will allow this:

class Foo(list[Bar]): ...
class Bar: ...

But it won't allow this, because there's no known use case for it:

class Foo(Bar): ...
class Bar: ...

(Ruff will allow ^both of those, but it's the same idea of "allowing pragmatic forward references, but not arbitrary forward references")

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(If it does get added to a spec somewhere or we think there is a good reason to support arbitrary forward references in stubs, I'm personally very open to adding support for it at Ruff.)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense in general; the second change in this PR (connect = Connection before Connection is defined) is definitely some very odd code.

I'm not sure why Ruff has an issue with the definition of def type_check_only, though. _F is used only in an annotation; why does the definition of type_check_only need to be after the definition of _F?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue in typing.pyi is that the TypeVar _F is bound to Callable before Callable is defined

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I guess that's the kind of case we only encounter in typeshed :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup! 😄


Any = object()

def final(f: _T) -> _T: ...
Expand Down Expand Up @@ -183,12 +180,6 @@ class _SpecialForm:
def __or__(self, other: Any) -> _SpecialForm: ...
def __ror__(self, other: Any) -> _SpecialForm: ...

_F = TypeVar("_F", bound=Callable[..., Any])
_P = _ParamSpec("_P")
_T = TypeVar("_T")

def overload(func: _F) -> _F: ...

Union: _SpecialForm
Generic: _SpecialForm
# Protocol is only present in 3.8 and later, but mypy needs it unconditionally
Expand Down Expand Up @@ -295,6 +286,10 @@ if sys.version_info >= (3, 10):
else:
def NewType(name: str, tp: Any) -> Any: ...

_F = TypeVar("_F", bound=Callable[..., Any])
_P = _ParamSpec("_P")
_T = TypeVar("_T")

# These type variables are used by the container types.
_S = TypeVar("_S")
_KT = TypeVar("_KT") # Key type.
Expand All @@ -304,9 +299,13 @@ _KT_co = TypeVar("_KT_co", covariant=True) # Key type covariant containers.
_VT_co = TypeVar("_VT_co", covariant=True) # Value type covariant containers.
_TC = TypeVar("_TC", bound=type[object])

def overload(func: _F) -> _F: ...
def no_type_check(arg: _F) -> _F: ...
def no_type_check_decorator(decorator: Callable[_P, _T]) -> Callable[_P, _T]: ...

# This itself is only available during type checking
def type_check_only(func_or_cls: _F) -> _F: ...

# Type aliases and type constructors

class _Alias:
Expand Down
3 changes: 2 additions & 1 deletion stubs/hdbcli/hdbcli/dbapi.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ from .resultrow import ResultRow
apilevel: str
threadsafety: int
paramstyle: tuple[str, ...] # hdbcli defines it as a tuple which does not follow PEP 249
connect = Connection

class Connection:
def __init__(
Expand Down Expand Up @@ -40,6 +39,8 @@ class Connection:
def setautocommit(self, auto: bool = ...) -> None: ...
def setclientinfo(self, key: str, value: str | None = ...) -> None: ...

connect = Connection

class LOB:
def __init__(self, *args: Any, **kwargs: Any) -> None: ...
def close(self) -> bool: ...
Expand Down