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

Initial pyright config #4192

Merged
merged 18 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
74 changes: 74 additions & 0 deletions .github/workflows/pyright.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# Split workflow file to not interfere with skeleton
name: pyright

on:
merge_group:
push:
branches-ignore:
# temporary GH branches relating to merge queues (jaraco/skeleton#93)
- gh-readonly-queue/**
tags:
# required if branches-ignore is supplied (jaraco/skeleton#103)
- '**'
pull_request:
workflow_dispatch:

concurrency:
group: >-
${{ github.workflow }}-
${{ github.ref_type }}-
${{ github.event.pull_request.number || github.sha }}
cancel-in-progress: true

env:
# pin pyright version so a new version doesn't suddenly cause the CI to fail,
# until types-setuptools is removed from typeshed.
# For help with static-typing issues, or pyright update, ping @Avasam
PYRIGHT_VERSION: "1.1.377"

# Environment variable to support color support (jaraco/skeleton#66)
FORCE_COLOR: 1

# Suppress noisy pip warnings
PIP_DISABLE_PIP_VERSION_CHECK: 'true'
PIP_NO_PYTHON_VERSION_WARNING: 'true'
PIP_NO_WARN_SCRIPT_LOCATION: 'true'

jobs:
pyright:
strategy:
# https://blog.jaraco.com/efficient-use-of-ci-resources/
matrix:
python:
- "3.8"
- "3.12"
platform:
- ubuntu-latest
runs-on: ${{ matrix.platform }}
timeout-minutes: 10
steps:
- uses: actions/checkout@v4
- name: Setup Python
uses: actions/setup-python@v5
with:
python-version: ${{ matrix.python }}
allow-prereleases: true
- name: Install typed dependencies
run: python -m pip install -e .[core,type]
- name: Inform how to run locally
run: |
echo 'To run this test locally with npm pre-installed, run:'
echo '> npx -y pyright@${{ env.PYRIGHT_VERSION }} --threads'
echo 'You can also instead install "Pyright for Python" which will install npm for you:'
if [ '$PYRIGHT_VERSION' == 'latest' ]; then
echo '> pip install -U'
else
echo '> pip install pyright==${{ env.PYRIGHT_VERSION }}'
fi
echo 'pyright --threads'
shell: bash
- name: Run pyright
uses: jakebailey/pyright-action@v2
with:
version: ${{ env.PYRIGHT_VERSION }}
extra-args: --threads
12 changes: 12 additions & 0 deletions pkg_resources/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -561,24 +561,30 @@ def get_entry_info(dist: _EPDistType, group: str, name: str) -> EntryPoint | Non
class IMetadataProvider(Protocol):
def has_metadata(self, name: str) -> bool:
"""Does the package's distribution contain the named metadata?"""
...

def get_metadata(self, name: str) -> str:
"""The named metadata resource as a string"""
...

def get_metadata_lines(self, name: str) -> Iterator[str]:
"""Yield named metadata resource as list of non-blank non-comment lines

Leading and trailing whitespace is stripped from each line, and lines
with ``#`` as the first non-blank character are omitted."""
...

def metadata_isdir(self, name: str) -> bool:
"""Is the named metadata a directory? (like ``os.path.isdir()``)"""
...

def metadata_listdir(self, name: str) -> list[str]:
"""List of metadata names in the directory (like ``os.listdir()``)"""
...

def run_script(self, script_name: str, namespace: dict[str, Any]) -> None:
"""Execute the named script in the supplied namespace dictionary"""
...


class IResourceProvider(IMetadataProvider, Protocol):
Expand All @@ -590,29 +596,35 @@ def get_resource_filename(
"""Return a true filesystem path for `resource_name`

`manager` must be a ``ResourceManager``"""
...

def get_resource_stream(
self, manager: ResourceManager, resource_name: str
) -> _ResourceStream:
"""Return a readable file-like object for `resource_name`

`manager` must be a ``ResourceManager``"""
...

def get_resource_string(
self, manager: ResourceManager, resource_name: str
) -> bytes:
"""Return the contents of `resource_name` as :obj:`bytes`

`manager` must be a ``ResourceManager``"""
...

def has_resource(self, resource_name: str) -> bool:
"""Does the package contain the named resource?"""
...

def resource_isdir(self, resource_name: str) -> bool:
"""Is the named resource a directory? (like ``os.path.isdir()``)"""
...

def resource_listdir(self, resource_name: str) -> list[str]:
"""List of resource names in the directory (like ``os.listdir()``)"""
...


class WorkingSet:
Expand Down
6 changes: 3 additions & 3 deletions pkg_resources/tests/test_pkg_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def teardown_class(cls):
finalizer()

def test_resource_listdir(self):
import mod
import mod # pyright: ignore[reportMissingImports] # Temporary package for test

zp = pkg_resources.ZipProvider(mod)

Expand All @@ -84,7 +84,7 @@ def test_resource_listdir(self):
assert zp.resource_listdir('nonexistent') == []
assert zp.resource_listdir('nonexistent/') == []

import mod2
import mod2 # pyright: ignore[reportMissingImports] # Temporary package for test

zp2 = pkg_resources.ZipProvider(mod2)

Expand All @@ -100,7 +100,7 @@ def test_resource_filename_rewrites_on_change(self):
same size and modification time, it should not be overwritten on a
subsequent call to get_resource_filename.
"""
import mod
import mod # pyright: ignore[reportMissingImports] # Temporary package for test

manager = pkg_resources.ResourceManager()
zp = pkg_resources.ZipProvider(mod)
Expand Down
8 changes: 4 additions & 4 deletions pkg_resources/tests/test_resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -817,11 +817,11 @@ def test_two_levels_deep(self, symlinked_tmpdir):
(pkg1 / '__init__.py').write_text(self.ns_str, encoding='utf-8')
(pkg2 / '__init__.py').write_text(self.ns_str, encoding='utf-8')
with pytest.warns(DeprecationWarning, match="pkg_resources.declare_namespace"):
import pkg1
import pkg1 # pyright: ignore[reportMissingImports] # Temporary package for test
assert "pkg1" in pkg_resources._namespace_packages
# attempt to import pkg2 from site-pkgs2
with pytest.warns(DeprecationWarning, match="pkg_resources.declare_namespace"):
import pkg1.pkg2
import pkg1.pkg2 # pyright: ignore[reportMissingImports] # Temporary package for test
# check the _namespace_packages dict
assert "pkg1.pkg2" in pkg_resources._namespace_packages
assert pkg_resources._namespace_packages["pkg1"] == ["pkg1.pkg2"]
Expand Down Expand Up @@ -862,8 +862,8 @@ def test_path_order(self, symlinked_tmpdir):
(subpkg / '__init__.py').write_text(vers_str % number, encoding='utf-8')

with pytest.warns(DeprecationWarning, match="pkg_resources.declare_namespace"):
import nspkg
import nspkg.subpkg
import nspkg # pyright: ignore[reportMissingImports] # Temporary package for test
import nspkg.subpkg # pyright: ignore[reportMissingImports] # Temporary package for test
expected = [str(site.realpath() / 'nspkg') for site in site_dirs]
assert nspkg.__path__ == expected
assert nspkg.subpkg.__version__ == 1
4 changes: 4 additions & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,10 @@ type = [
# until types-setuptools is removed from typeshed.
# For help with static-typing issues, or mypy update, ping @Avasam
"mypy==1.11.*",
# Typing fixes in version newer than we require at runtime
"importlib_metadata>=7.0.2; python_version < '3.10'",
# Imported unconditionally in tools/finalize.py
'jaraco.develop >= 7.21; sys_platform != "cygwin"',
]


Expand Down
32 changes: 32 additions & 0 deletions pyrightconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"$schema": "https://raw.githubusercontent.com/microsoft/pyright/main/packages/vscode-pyright/schemas/pyrightconfig.schema.json",
"exclude": [
"build",
".tox",
".eggs",
"**/_vendor", // Vendored
"setuptools/_distutils", // Vendored
"setuptools/config/_validate_pyproject/**", // Auto-generated
],
// Our testing setup doesn't allow passing CLI arguments, so local devs have to set this manually.
// "pythonVersion": "3.8",
// For now we don't mind if mypy's `type: ignore` comments accidentally suppresses pyright issues
"enableTypeIgnoreComments": true,
"typeCheckingMode": "basic",
// Too many issues caused by dynamic patching, still worth fixing when we can
"reportAttributeAccessIssue": "warning",
// Fails on Python 3.12 due to missing distutils and on cygwin CI tests
"reportAssignmentType": "warning",
"reportMissingImports": "warning",
"reportOptionalCall": "warning",
// FIXME: A handful of reportOperatorIssue spread throughout the codebase
"reportOperatorIssue": "warning",
// Deferred initialization (initialize_options/finalize_options) causes many "potentially None" issues
// TODO: Fix with type-guards or by changing how it's initialized
"reportArgumentType": "warning", // A lot of these are caused by jaraco.path.build's spec argument not being a Mapping https://github.com/jaraco/jaraco.path/pull/3
Avasam marked this conversation as resolved.
Show resolved Hide resolved
"reportCallIssue": "warning",
"reportGeneralTypeIssues": "warning",
"reportOptionalIterable": "warning",
"reportOptionalMemberAccess": "warning",
"reportOptionalOperand": "warning",
}
3 changes: 2 additions & 1 deletion setuptools/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import re
import sys
from abc import abstractmethod
from collections.abc import Mapping
from typing import TYPE_CHECKING, TypeVar, overload

sys.path.extend(((vendor_path := os.path.join(os.path.dirname(os.path.dirname(__file__)), 'setuptools', '_vendor')) not in sys.path) * [vendor_path]) # fmt: skip
Expand Down Expand Up @@ -59,7 +60,7 @@ class MinimalDistribution(distutils.core.Distribution):
fetch_build_eggs interface.
"""

def __init__(self, attrs):
def __init__(self, attrs: Mapping[str, object]):
_incl = 'dependency_links', 'setup_requires'
filtered = {k: attrs[k] for k in set(_incl) & set(attrs)}
super().__init__(filtered)
Expand Down
8 changes: 3 additions & 5 deletions setuptools/_reqs.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,13 @@ def parse_strings(strs: _StrOrIter) -> Iterator[str]:
return text.join_continuation(map(text.drop_comment, text.yield_lines(strs)))


# These overloads are only needed because of a mypy false-positive, pyright gets it right
# https://github.com/python/mypy/issues/3737
@overload
def parse(strs: _StrOrIter) -> Iterator[Requirement]: ...


@overload
def parse(strs: _StrOrIter, parser: Callable[[str], _T]) -> Iterator[_T]: ...


def parse(strs, parser=parse_req):
def parse(strs: _StrOrIter, parser: Callable[[str], _T] = parse_req) -> Iterator[_T]: # type: ignore[assignment]
"""
Replacement for ``pkg_resources.parse_requirements`` that uses ``packaging``.
"""
Expand Down
6 changes: 6 additions & 0 deletions setuptools/command/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,15 @@ def finalize_options(self):

def initialize_options(self):
"""(Required by the original :class:`setuptools.Command` interface)"""
...

def finalize_options(self):
"""(Required by the original :class:`setuptools.Command` interface)"""
...

def run(self):
"""(Required by the original :class:`setuptools.Command` interface)"""
...

def get_source_files(self) -> list[str]:
"""
Expand All @@ -104,6 +107,7 @@ def get_source_files(self) -> list[str]:
with all the files necessary to build the distribution.
All files should be strings relative to the project root directory.
"""
...

def get_outputs(self) -> list[str]:
"""
Expand All @@ -117,6 +121,7 @@ def get_outputs(self) -> list[str]:
in ``get_output_mapping()`` plus files that are generated during the build
and don't correspond to any source file already present in the project.
"""
...

def get_output_mapping(self) -> dict[str, str]:
"""
Expand All @@ -127,3 +132,4 @@ def get_output_mapping(self) -> dict[str, str]:
Destination files should be strings in the form of
``"{build_lib}/destination/file/path"``.
"""
...
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Python should not require ellipsis when the docstring is provided. I am not 100% inline with the comment of the pyright maintainer... The reason why the linters don't complain about it, is because there is nothing wrong 😅

Does mypy also complains about it?
Should we just stick with mypy for now?

Copy link
Contributor

Choose a reason for hiding this comment

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

I see the problem here is because we are talking specifically about prototypes, and the spec says:

Bodies of protocol methods are type checked.

That is a bit disappointing... These pesky ellipsis can be annoying

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Further discussion: astral-sh/ruff#8756 (comment) . It does seem to be more accurate to spec. Ruff was also updated to not strip ellipses in Protocols in https://github.com/astral-sh/ruff/releases/tag/v0.1.6

4 changes: 3 additions & 1 deletion setuptools/command/build_clib.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
from distutils.errors import DistutilsSetupError

try:
from distutils._modified import newer_pairwise_group
from distutils._modified import ( # pyright: ignore[reportMissingImports]
newer_pairwise_group,
)
except ImportError:
# fallback for SETUPTOOLS_USE_DISTUTILS=stdlib
from .._distutils._modified import newer_pairwise_group
Expand Down
6 changes: 4 additions & 2 deletions setuptools/command/easy_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -1792,7 +1792,7 @@ def auto_chmod(func, arg, exc):
return func(arg)
et, ev, _ = sys.exc_info()
# TODO: This code doesn't make sense. What is it trying to do?
raise (ev[0], ev[1] + (" %s %s" % (func, arg)))
raise (ev[0], ev[1] + (" %s %s" % (func, arg))) # pyright: ignore[reportOptionalSubscript, reportIndexIssue]


def update_dist_caches(dist_path, fix_zipimporter_caches):
Expand Down Expand Up @@ -2018,7 +2018,9 @@ def is_python_script(script_text, filename):


try:
from os import chmod as _chmod
from os import (
chmod as _chmod, # pyright: ignore[reportAssignmentType] # Loosing type-safety w/ pyright, but that's ok
)
except ImportError:
# Jython compatibility
def _chmod(*args: object, **kwargs: object) -> None: # type: ignore[misc] # Mypy re-uses the imported definition anyway
Expand Down
4 changes: 3 additions & 1 deletion setuptools/command/editable_wheel.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@
from .install_scripts import install_scripts as install_scripts_cls

if TYPE_CHECKING:
from typing_extensions import Self

from .._vendor.wheel.wheelfile import WheelFile

_P = TypeVar("_P", bound=StrPath)
Expand Down Expand Up @@ -379,7 +381,7 @@ def _select_strategy(
class EditableStrategy(Protocol):
def __call__(self, wheel: WheelFile, files: list[str], mapping: dict[str, str]): ...

def __enter__(self): ...
def __enter__(self) -> Self: ...

def __exit__(self, _exc_type, _exc_value, _traceback): ...

Expand Down
8 changes: 6 additions & 2 deletions setuptools/config/pyprojecttoml.py
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,10 @@ def _obtain(self, dist: Distribution, field: str, package_dir: Mapping[str, str]
def _obtain_version(self, dist: Distribution, package_dir: Mapping[str, str]):
# Since plugins can set version, let's silently skip if it cannot be obtained
if "version" in self.dynamic and "version" in self.dynamic_cfg:
return _expand.version(self._obtain(dist, "version", package_dir))
return _expand.version(
# We already do an early check for the presence of "version"
self._obtain(dist, "version", package_dir) # pyright: ignore[reportArgumentType]
)
return None

def _obtain_readme(self, dist: Distribution) -> dict[str, str] | None:
Expand All @@ -313,9 +316,10 @@ def _obtain_readme(self, dist: Distribution) -> dict[str, str] | None:
dynamic_cfg = self.dynamic_cfg
if "readme" in dynamic_cfg:
return {
# We already do an early check for the presence of "readme"
"text": self._obtain(dist, "readme", {}),
"content-type": dynamic_cfg["readme"].get("content-type", "text/x-rst"),
}
} # pyright: ignore[reportReturnType]

self._ensure_previously_set(dist, "readme")
return None
Expand Down
Loading
Loading