Skip to content

type setuptools.extension.Extension #5022

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

type setuptools.extension.Extension #5022

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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 32 additions & 19 deletions setuptools/extension.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@
from .monkey import get_unpatched

import distutils.core
import distutils.errors
import distutils.extension


def _have_cython() -> bool:
Expand Down Expand Up @@ -142,23 +140,38 @@ class Extension(_Extension):
_needs_stub: bool #: Private API, internal use only.
_file_name: str #: Private API, internal use only.

def __init__(
self,
name: str,
sources: Iterable[StrPath],
*args,
py_limited_api: bool = False,
**kw,
) -> None:
# The *args is needed for compatibility as calls may use positional
# arguments. py_limited_api may be set only via keyword.
self.py_limited_api = py_limited_api
super().__init__(
name,
sources, # type: ignore[arg-type] # Vendored version of setuptools supports PathLike
*args,
**kw,
)
if TYPE_CHECKING:
# Same as distutils.extension.Extension, with an extra py_limited_api kwarg,
# and without **kw meant to catch unknown keywords
def __init__(
self,
name: str,
sources: Iterable[StrPath],
include_dirs: list[str] | None = None,
define_macros: list[tuple[str, str | None]] | None = None,
undef_macros: list[str] | None = None,
library_dirs: list[str] | None = None,
libraries: list[str] | None = None,
runtime_library_dirs: list[str] | None = None,
extra_objects: list[str] | None = None,
extra_compile_args: list[str] | None = None,
extra_link_args: list[str] | None = None,
export_symbols: list[str] | None = None,
swig_opts: list[str] | None = None,
depends: list[str] | None = None,
language: str | None = None,
optional: bool | None = None,
*,
py_limited_api: bool = False,
) -> None: ...
Copy link
Contributor

Choose a reason for hiding this comment

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

It is a bit disappointing that we have to repeat everything from distutils.Extension.
Ideally it would be nice to rely on single source of truth, and that would also improve maintenance (e.g. if something changes in distutils we need to change it in setuptools).

So I was wondering...

If (and that is a big if) we managed to coordinate this step with distutils, could we use PEP 692 to simplify this?

For example, let's suppose we manage to define class _ExtensionArgs(TypedDict) in distutils.extension. Then it would be very lightweight to either use it in setuptools or inherit from it.

Does it make sense?

/cc @jaraco

Copy link
Contributor Author

@Avasam Avasam Jun 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Since this doesn't need to expose extra kwargs (unlike #5021) this can be done today.
You'd also need a tuple type to unpack for args. BUT, I don't think you can mark optional arguments there.
You could consider positional args here "unsafe"/"barely supported"/"deprecated"/whatever, so you'd only have to type the union of all possible arguments, (easy to copy/past and let Ruff simplify)

AFAIK, there's no way to extend kwargs from another function in Python's type system (it's possible for positional args since you can use Concatenate with ParamSpec.args)

It would look like this:

class _ExtensionOptionalKWArgs(TypedDict, total=False):
    include_dirs: list[str] | None
    define_macros: list[tuple[str, str | None]] | None
    undef_macros: list[str] | None
    library_dirs: list[str] | None
    libraries: list[str] | None
    runtime_library_dirs: list[str] | None
    extra_objects: list[str] | None
    extra_compile_args: list[str] | None
    extra_link_args: list[str] | None
    export_symbols: list[str] | None
    swig_opts: list[str] | None
    depends: list[str] | None
    language: str | None
    optional: bool | None

_ExtensionOptionalArgs: TypeAlias = (
    list[str] | None | list[tuple[str, str | None]] | bool | str
)

class Extension(_Extension):
    def __init__(
        self,
        name: str,
        sources: Iterable[str | os.PathLike[str]],
        *args: _ExtensionOptionalArgs,
        py_limited_api: bool = False,
        **kw: Unpack[_ExtensionOptionalKWArgs],
    ) -> None:
        # The *args is needed for compatibility as calls may use positional
        # arguments. py_limited_api may be set only via keyword.
        self.py_limited_api = py_limited_api
        super().__init__(name, sources, *args, **kw)  # type: ignore[arg-type]

Compared to the current solution:

  • Cons:
    • You loose information on the default values (doesn't affect type-checking, informational only)
    • More book keeping for distutils (inline function annotation + args union + kwargs TypedDictDict). Granted it's easy to copy-paste.
    • Less type-safe positional-arguments
    • An additional type-ignore in setuptools
    • Either you still have to copy 2 annotations from distutils anyway (for name and sources), or you have to loose the fact that in positional-only, these two are required. (from a typing pov)
  • Pros:
    • Setuptools doesn't have to directly copy-paste (without modifications) a signature from distutils. (which only affects typing, not runtime)

Oh and type-checkers already ensure that the setuptools override stays compatible with the base distutils type (well, up to Python 3.11 and the stdlib version only until #4689 lands)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @Avasam for looking into it.

AFAIK, there's no way to extend kwargs from another function in Python's type system (it's possible for positional args since you can use Concatenate with ParamSpec.args)

Yeah, that is always a struggle... I went back to read PEP 612 and I noticed that they explicitly excluded that from the standard :(
I understand it is difficult to implement but it would be very useful. I did not know PEP 728, hope it gets passed.

An additional type-ignore in setuptools

If both _ExtensionOptionalKWArgs and _ExtensionOptionalArgs are defined in distutils and used for the definition of distutils.extension.Extension, would that still be required?

You'd also need a tuple type to unpack for args. BUT, I don't think you can mark optional arguments there.
You could consider positional args here "unsafe"/"barely supported"/"deprecated"/whatever, so you'd only have to type the union of all possible arguments, (easy to copy/past and let Ruff simplify)

I think that is the major crux with this alternative approach. If we define distutils.extension.Extension.__init__(self, *args: _ExtensionOptionalArgs, **kwargs: Unpack[_ExtensionOptionalKWArgs]), this would require some effort to keep backwards compatibility1 and we may loose type safety for positional arguments.

More book keeping for distutils (inline function annotation + args union + kwargs TypedDictDict). Granted it's easy to copy-paste.

Ideally if we are going for that both signatures of setuptools and distutils should use _ExtensionOptionalKWArgs so that we don't have duplication of the definitions. So the extra churn in distutils is likely to be: definining _ExtensionOptionalKWArgs & _ExtensionOptionalArgs, and refactoring the code to use them, right?

There is a problem with this approach though.
I don't know if the type checkers are smart enough to understand something like:

# kwargs: _ExtensionOptionalKWArgs

self.runtime_library_dirs = kwargs.get('runtime_library_dirs')

and correctly infer the type of self.runtime_library_dirs. That could limit the viability of this option because it would require duplication again.

Footnotes

  1. e.g. zipping args with _ExtensionOptionalKWArgs.__annotations__.keys into a dict and then merging it with kwargs before processing kwargs. ↩

Copy link
Contributor

Choose a reason for hiding this comment

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

For the sake of brainstorm I also messed around with:

https://gist.github.com/mypy-play/34021eebf754aa542f69631eed2f0dbd

Copy link
Contributor Author

@Avasam Avasam Jul 1, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I don't know if the type checkers are smart enough to understand something like:

# kwargs: _ExtensionOptionalKWArgs

self.runtime_library_dirs = kwargs.get('runtime_library_dirs')

and correctly infer the type of self.runtime_library_dirs. That could limit the viability of this option because it would require duplication again.

I'm pleasantly surprised to see mypy support it: https://mypy-play.net/?mypy=latest&python=3.12&gist=b2112018a555537c2c67e212f7b98018

So does pyright (less surprised): Playground link

Even Pyrefly: Sandbox link

I can't imagine upcoming Astral's ty wouldn't (it's just not complete enough yet)

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @Avasam, I have opened pypa/distutils#373 on what would be my preference for this specific problem.


else:

def __init__(self, *args, py_limited_api: bool = False, **kw) -> None:
# The *args is needed for compatibility as calls may use positional
# arguments. py_limited_api may be set only via keyword.
self.py_limited_api = py_limited_api
super().__init__(*args, **kw)

def _convert_pyx_sources_to_lang(self):
"""
Expand Down
Loading