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.

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

Give Pyright what it wants (alias attributes everywhere) #3114

Merged
merged 30 commits into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
deb08a3
Pyright wants aliases everywhere
A5rocks Oct 19, 2024
1e65f68
Changelog
A5rocks Oct 19, 2024
59abcf8
Make backslash replacement more correct
A5rocks Oct 19, 2024
cbb08fb
Debug which class doesn't exist
A5rocks Oct 19, 2024
9c97bad
Debug a bit better
A5rocks Oct 19, 2024
35609d4
Fix weirdness around PosixPath needing to be resolved
A5rocks Oct 19, 2024
4bc3e58
Appease type checker
A5rocks Oct 19, 2024
37b9c38
Don't even consider tests in the alias test
A5rocks Oct 20, 2024
3cda304
Turn paths into strings before indexing
A5rocks Oct 20, 2024
9eca851
Explain the test better
A5rocks Oct 20, 2024
ba67f7e
Fixes for pre-commit
A5rocks Oct 20, 2024
88cf458
Reformat according to black
A5rocks Oct 20, 2024
819797a
One last pre-commit fix
A5rocks Oct 20, 2024
f21cbb0
Simplify alias test by monkeypatching attrs.
TeamSpen210 Oct 22, 2024
30befc9
Skip pyright init attributes test if plugin has not run
TeamSpen210 Nov 10, 2024
a23712c
Catch any other renaming too
A5rocks Nov 10, 2024
74e67f1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Nov 10, 2024
593701b
Handle autoattribs
A5rocks Nov 10, 2024
ff487e2
Disambiguate
A5rocks Nov 10, 2024
a6ea148
Synchronise name
TeamSpen210 Nov 10, 2024
637cc5f
Update import mode because it worked locally
A5rocks Nov 24, 2024
d8c0983
Check suspicions
A5rocks Nov 24, 2024
f32b8fb
Double check
A5rocks Nov 24, 2024
7ccead5
Don't rely on installing `_trio_check_attrs_aliases.py` as a module
A5rocks Nov 24, 2024
9b4c585
importlib import mode means we can use conftest again!
A5rocks Nov 24, 2024
0c1ca17
Just do something hacky instead
A5rocks Nov 24, 2024
f0294da
Debug and remove `-Wall` for later PR
A5rocks Nov 24, 2024
cb5913d
Hopefully fix some things
A5rocks Nov 24, 2024
942b371
Give into perl
A5rocks Nov 24, 2024
ce806e9
Add perl to alpine (should work now)
A5rocks Nov 24, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ jobs:
# can't use setup-python because that python doesn't seem to work;
# `python3-dev` (rather than `python:alpine`) for some ctypes reason,
# `nodejs` for pyright (`node-env` pulls in nodejs but that takes a while and can time out the test).
run: apk update && apk add python3-dev bash nodejs
# `perl` for a platform independent `sed -i` alternative
run: apk update && apk add python3-dev bash nodejs perl
- name: Enter virtual environment
run: python -m venv .venv
- name: Run tests
Expand Down
14 changes: 10 additions & 4 deletions ci.sh
Original file line number Diff line number Diff line change
Expand Up @@ -116,23 +116,29 @@ else
echo "::group::Setup for tests"

# We run the tests from inside an empty directory, to make sure Python
# doesn't pick up any .py files from our working dir. Might have been
# pre-created by some of the code above.
# doesn't pick up any .py files from our working dir. Might have already
# been created by a previous run.
mkdir empty || true
cd empty

INSTALLDIR=$(python -c "import os, trio; print(os.path.dirname(trio.__file__))")
cp ../pyproject.toml "$INSTALLDIR"
cp ../pyproject.toml "$INSTALLDIR" # TODO: remove this

# get mypy tests a nice cache
MYPYPATH=".." mypy --config-file= --cache-dir=./.mypy_cache -c "import trio" >/dev/null 2>/dev/null || true

# support subprocess spawning with coverage.py
echo "import coverage; coverage.process_startup()" | tee -a "$INSTALLDIR/../sitecustomize.py"

perl -i -pe 's/-p trio\._tests\.pytest_plugin//' "$INSTALLDIR/pyproject.toml"

echo "::endgroup::"
echo "::group:: Run Tests"
if COVERAGE_PROCESS_START=$(pwd)/../pyproject.toml coverage run --rcfile=../pyproject.toml -m pytest -ra --junitxml=../test-results.xml --run-slow "${INSTALLDIR}" --verbose --durations=10 $flags; then
if PYTHONPATH=../tests COVERAGE_PROCESS_START=$(pwd)/../pyproject.toml \
coverage run --rcfile=../pyproject.toml -m \
pytest -ra --junitxml=../test-results.xml \
-p _trio_check_attrs_aliases --verbose --durations=10 \
-p trio._tests.pytest_plugin --run-slow $flags "${INSTALLDIR}"; then
PASSED=true
else
PASSED=false
Expand Down
1 change: 1 addition & 0 deletions newsfragments/3114.bugfix.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Ensure that Pyright recognizes our underscore prefixed attributes for attrs classes.
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ reportUnnecessaryTypeIgnoreComment = true
typeCheckingMode = "strict"

[tool.pytest.ini_options]
addopts = ["--strict-markers", "--strict-config", "-p trio._tests.pytest_plugin"]
addopts = ["--strict-markers", "--strict-config", "-p trio._tests.pytest_plugin", "--import-mode=importlib"]
faulthandler_timeout = 60
filterwarnings = [
"error",
Expand Down
4 changes: 2 additions & 2 deletions src/trio/_core/_local.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ class RunVar(Generic[T]):

"""

_name: str
_default: T | type[_NoValue] = _NoValue
_name: str = attrs.field(alias="name")
_default: T | type[_NoValue] = attrs.field(default=_NoValue, alias="default")

def get(self, default: T | type[_NoValue] = _NoValue) -> T:
"""Gets the value of this :class:`RunVar` for the current run call."""
Expand Down
10 changes: 7 additions & 3 deletions src/trio/_core/_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -544,9 +544,13 @@ class CancelScope:
cancelled_caught: bool = attrs.field(default=False, init=False)

# Constructor arguments:
_relative_deadline: float = attrs.field(default=inf, kw_only=True)
_deadline: float = attrs.field(default=inf, kw_only=True)
_shield: bool = attrs.field(default=False, kw_only=True)
_relative_deadline: float = attrs.field(
default=inf,
kw_only=True,
alias="relative_deadline",
)
_deadline: float = attrs.field(default=inf, kw_only=True, alias="deadline")
_shield: bool = attrs.field(default=False, kw_only=True, alias="shield")

def __attrs_post_init__(self) -> None:
if isnan(self._deadline):
Expand Down
2 changes: 1 addition & 1 deletion src/trio/_core/_tests/tutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@

import pytest

# See trio/_tests/conftest.py for the other half of this
# See trio/_tests/pytest_plugin.py for the other half of this
from trio._tests.pytest_plugin import RUN_SLOW

if TYPE_CHECKING:
Expand Down
37 changes: 35 additions & 2 deletions src/trio/_tests/test_exports.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,10 @@

import trio
import trio.testing
from trio._tests.pytest_plugin import skip_if_optional_else_raise
from trio._tests.pytest_plugin import RUN_SLOW, skip_if_optional_else_raise

from .. import _core, _util
from .._core._tests.tutil import slow
from .pytest_plugin import RUN_SLOW

if TYPE_CHECKING:
from collections.abc import Iterable, Iterator
Expand Down Expand Up @@ -572,3 +571,37 @@ def test_classes_are_final() -> None:
continue

assert class_is_final(class_)


# Plugin might not be running, especially if running from an installed version.
@pytest.mark.skipif(
not hasattr(attrs.field, "trio_modded"),
reason="Pytest plugin not installed.",
)
def test_pyright_recognizes_init_attributes() -> None:
"""Check whether we provide `alias` for all underscore prefixed attributes.

Attrs always sets the `alias` attribute on fields, so a pytest plugin is used
to monkeypatch `field()` to record whether an alias was defined in the metadata.
See `_trio_check_attrs_aliases`.
"""
for module in PUBLIC_MODULES:
for class_ in module.__dict__.values():
if not attrs.has(class_):
continue
if isinstance(class_, _util.NoPublicConstructor):
continue

attributes = [
attr
for attr in attrs.fields(class_)
if attr.init
if attr.alias
not in (
attr.name,
# trio_original_args may not be present in autoattribs
attr.metadata.get("trio_original_args", {}).get("alias"),
)
]

assert attributes == [], class_
22 changes: 22 additions & 0 deletions tests/_trio_check_attrs_aliases.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
"""Plugins are executed by Pytest before test modules.

We use this to monkeypatch attrs.field(), so that we can detect if aliases are used for test_exports.
"""

from typing import Any

import attrs

orig_field = attrs.field


def field(**kwargs: Any) -> Any:
original_args = kwargs.copy()
metadata = kwargs.setdefault("metadata", {})
metadata["trio_original_args"] = original_args
return orig_field(**kwargs)


# Mark it as being ours, so the test knows it can actually run.
field.trio_modded = True # type: ignore
attrs.field = field
Loading