| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
|
|
||
| __version__: str | ||
|
|
||
| class _Library(TypedDict): |
There was a problem hiding this comment.
Should this be marked @type_check_only? There seems to be some precedence for using it on some similar typeddicts, e.g.
Lines 5 to 6 in 7ccc587
Also, to bikeshed the name, I see setuptools internally calls this structure a build_info in a few places, so maybe _BuildInfo would be a good fit?
Sorry, something went wrong.
There was a problem hiding this comment.
Interesting, I wasn't aware of it, but likely what we want here. Thanks for the callout. Agree that's a better name here.
Sorry, something went wrong.
There was a problem hiding this comment.
Sorry, something went wrong.
| @type_check_only | ||
| class _BuildInfo(TypedDict): | ||
| sources: list[str] | tuple[str] | ||
| obj_deps: NotRequired[dict[str, list[str] | tuple[str]]] | ||
| macros: NotRequired[list[tuple[str, str] | tuple[str]]] | ||
| include_dirs: NotRequired[list[str]] | ||
| cflags: NotRequired[list[str]] |
There was a problem hiding this comment.
Normally I'd prefer defining such types close to where their implementation stands (so with the appropriate Command or Distribution) but in this case it'd end up in distutils and/or setuptools/_distutils so I guess in setuptools.__init__.py is fine.
I do have a concern using a type-only TypedDict with required members as a parameter: It makes it annoying to use a non-literal dict. Maybe that's ok in setuptools due to the nature of it looking like a config file, it shouldn't be much more complex than my example below?
It's possible, but requires a private type-only import, which isn't great:
from typing import cast, TYPE_CHECKING
if TYPE_CHECKING:
from setuptools import _BuildInfo
libs = [
("lib1", {"sources": []}),
("lib2", {"sources": []}),
]
setup(libraries=libs) # Type error in both mypy and pyright due to missing "sources"
# Ok, but must quote type to avoid runtime error
setup(libraries=cast(list[tuple[str, "_BuildInfo"]], libs))
# Ok
setup(libraries=[("literal", {"sources": []})])
# Ok
libs_annotated: list[tuple[str, "_BuildInfo"]] = [ # Must quote type or use from __future__ import annotations
("lib1", {"sources": []}),
("lib2", {"sources": []}),
]
setup(libraries=libs_annotated)
Sorry, something went wrong.
There was a problem hiding this comment.
Yeah, perhaps it's not ideal but I'm not seeing a great alternative, obviously passing things in-line solves this problem, or just:
libs: "list[tuple[str, _BuildInfo]]" = [
("lib1", {"sources": []}),
("lib2", {"sources": []}),
]
setup(libraries=libs)Which would work without quotes given deferred annotations.
Sorry, something went wrong.
There was a problem hiding this comment.
At the same time, the kind of people to type-check their setup.py are probably the same going all out on typing (looking at myself here ^^)
Sorry, something went wrong.
Co-authored-by: Avasam <samuel.06@hotmail.com>
|
|
||
| @type_check_only | ||
| class _BuildInfo(TypedDict): | ||
| sources: list[str] | tuple[str] |
There was a problem hiding this comment.
Are you sure this isn't tuple[str, ...]? In other words, is this really always a one-tuple? (A few more times below.)
Sorry, something went wrong.
There was a problem hiding this comment.
You're right, I also glanced over that...
(same for all definitions below)
Sorry, something went wrong.
There was a problem hiding this comment.
The one tuple for macros is correct, but the others were a mistake: 2bb4ed4
Sorry, something went wrong.
There was a problem hiding this comment.
Oops, I also glanced over this: tuple[str] means a single-item tuple, we want tuple[str, ...]
https://github.com/python/typeshed/pull/12791/files#r1797961597
Sorry, something went wrong.
Yep, I assumed they were actually one-tuples from the original issue but it looks like macros is the only one that should be: https://github.com/python/typeshed/pull/12791/files#r1798508136 |
Sorry, something went wrong.
Additionally the second value in the tuple can be None: 82ad6cb Here's a good example of what the types are allowed to be at runtime. |
Sorry, something went wrong.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
Sorry, something went wrong.
this is necessary to have python/typeshed#12791
this is necessary to have python/typeshed#12791
this is necessary to have python/typeshed#12791
| Back | FazBrowse Home | New Git URL |
Fixes #12752