Run stubtest in the type-checking CI job - #1116
Conversation
Merging this PR will improve performance by 10.44%
Performance Changes
Tip Curious why this is faster? Comment Comparing Footnotes
|
| # mark "/") are correct. json.format is the one callable that really accepts a | ||
| # keyword, and its stub is fixed instead of allowlisted. | ||
| msgspec\.json\.encode | ||
| msgspec\.json\.decode |
There was a problem hiding this comment.
It is typed as:
def decode(
buf: Buffer | str,
/,
*,
type: type[_T],
strict: bool = True,
dec_hook: _DecHookSig = None,
) -> _T: ...It's real signature:
>>> msgspec.json.decode('{}')
{}
>>> msgspec.json.decode(buf='{}')
Traceback (most recent call last):
File "<python-input-5>", line 1, in <module>
msgspec.json.decode(buf='{}')
~~~~~~~~~~~~~~~~~~~^^^^^^^^^^
TypeError: Missing 1 required argumentsIt's __text_signature__:
>>> inspect.signature(msgspec.json.decode)
<Signature (buf, *, type='Any', strict=True, dec_hook=None)>
>>> msgspec.json.decode.__text_signature__
"(buf, *, type='Any', strict=True, dec_hook=None)"The correct fix would be to add / to the text signature:
"json_decode(buf, /, *, type='Any', strict=True, dec_hook=None)\n"And remove these entries from the stubtest ignore.
There was a problem hiding this comment.
Done in d4560b2: added the / to the __text_signature__ for json/msgpack encode/decode and the Encoder/Decoder methods, so inspect.signature reports them positional-only and stubtest agrees. Dropped all 10 pos-only entries from the allowlist. Behaviour is unchanged (they already rejected keyword calls), only the introspection metadata was wrong.
| msgspec\.Struct\.__init_subclass__ | ||
|
|
||
| # StructMeta is a disjoint base at runtime. Marking it @disjoint_base in the | ||
| # stub is deferred (see #1056) pending a typing_extensions compat decision. |
There was a problem hiding this comment.
Sorry, I don't understand :)
What compat decision are you talking about specifically?
That not all type checkers support @disjoint_base?
In this case it would be just a regular (T) -> T function.
There was a problem hiding this comment.
Right, my "compat decision" was just the worry that disjoint_base is new enough (typing_extensions 4.13) that pulling it into the stub raises the minimum type-checker/typeshed floor: an older mypy would error on the import rather than treat it as identity. But the stub already imports Buffer/Self/dataclass_transform from typing_extensions, so the floor is modern anyway and the bump is small. Added @disjoint_base to StructMeta in d4560b2 and dropped the allowlist entry. mypy and pyrefly are both happy with it.
If this is the case, you can use per-version ignores. See https://github.com/typeddjango/django-stubs/blob/master/scripts/stubtest.sh |
|
@sobolevn On per-version allowlists: django-stubs runs stubtest across a pinned matrix of Python versions, so a If you'd rather keep the unused-entry safety that |
|
Heads up on the red CI here: the failure is in the Fixed separately in #1121. Once that lands I will rebase this and CI goes green. |
Add the positional-only "/" to the encode/decode __text_signature__ strings in _core.c so introspection matches the runtime (these already reject keyword calls), letting stubtest drop the 10 pos-only allowlist entries. Mark StructMeta @disjoint_base in the stub to drop that entry too. The remaining allowlist keeps --ignore-unused-allowlist since stubtest runs on a single floating Python version, not a matrix.
Follow-up to msgspec#1028. msgspec#1028 introduced `none_member: mi.Type | None` in the union branch of `_json_schema.py` and guarded the `self.to_schema(none_member)` calls with a separate `has_none` boolean. mypy can't narrow `none_member` through the flag, so it errors: ``` _json_schema.py:363: error: Argument 1 to "to_schema" of "_SchemaGenerator" has incompatible type "Type | None"; expected "Type" [arg-type] ``` This is not caught by current CI because the msgspec source is not type-checked there (only `tests/typing` is); msgspec#1116 (adding stubtest) is what surfaces it. Fixing it here so msgspec#1116 can go green. The fix drops the redundant `has_none` flag and checks `none_member is not None` at the call sites, which gives mypy the narrowing. No runtime behavior change (`has_none` and `none_member` were always set together): `tests/unit/test_schema.py` still passes (115), and stubtest with mypy 2.2.0 is clean. Co-authored-by: Siyet <Siyet@users.noreply.github.com>
| env-run "test" "pyrefly check tests/typing" | ||
| ) ( | ||
| env-run "test" | ||
| "python -m mypy.stubtest msgspec --allowlist tests/typing/stubtest_allowlist.txt --ignore-unused-allowlist" |
There was a problem hiding this comment.
I still advocate for the version-based allowlists, because otherwise we would have --ignore-unused-allowlist option enabled, which can hide problems :(
Closes #1056.
Adds a
stubteststep to thetest-typingjob so stub/runtime drift is caught in CI instead of by hand (as in #1043, #1062).mypyis already in thetest-typinggroup, so no new dependency.Root-cause fixes (instead of allowlisting)
__text_signature__.json/msgpackencode/decodeand theEncoder/Decodermethods take their first arg positional-only at runtime, but their C__text_signature__omitted the/, soinspect.signaturereported them as positional-or-keyword and stubtest flagged 10 mismatches. Added the/in_core.cso introspection matches runtime; behaviour is unchanged (keyword calls already raised). This removes all 10 pos-only allowlist entries.StructMeta@disjoint_base.StructMetais a disjoint base at runtime; marked it@disjoint_basein the stub (the stub already imports fromtyping_extensions, so no new floor of consequence). Removes that allowlist entry.Other stub fixes
structs.pyiwas missing__all__.json.formats stub markedbufpositional-only (/), but the runtime accepts it as a keyword, so the/is dropped to match (this is the one callable that really takes a keyword).Allowlist
What is left in
tests/typing/stubtest_allowlist.txtis 10 structural false positives stubtest cannot see through: the per-class__init__synthesized in C for everyStruct/inspecttype, the custom__new__onMeta/Ext/UnsetType,Struct.__init_subclass__(config kwargs viadataclass_transform, surfaces on 3.14+), and the private_utilsmodule.Run with
--ignore-unused-allowlist. Thetest-typingjob runs stubtest on a single Python resolved frompython-version-file: pyproject.toml(currently 3.14, floats up over time), not a pinned matrix, so a single superset allowlist with--ignore-unusedis robust to the runner Python moving. Verified green on 3.12, 3.14, 3.15.