Run stubtest in the type-checking CI job - #1116
Conversation
Merging this PR will degrade performance by 97.81%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | test_import_time[arm-msgspec.structs] |
140.1 µs | 326,990 µs | -99.96% |
| ⚡ | Simulation | test_pickle_load[arm] |
1.8 ms | 1.6 ms | +11.48% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing add-stubtest-ci (07e1cad) with main (f51f378)
Footnotes
-
135 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
| # 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 :(
# Conflicts: # src/msgspec/__init__.pyi
Drops --ignore-unused-allowlist so an entry that stops matching fails the run. scripts/stubtest.py picks the common allowlist plus every stubtest_allowlist_py<major><minor>_plus.txt at or below the running interpreter.
test-typing now runs scripts/stubtest.py, so a change to it must run CI.
|
@sobolevn conceded on the per-version allowlists. Layout: Correction to my earlier comment: Verified on 3.10 through 3.14: clean, and a stale entry, a version-gated entry in the common file, or a missing per-version file each fail the run. One caveat worth stating: stubtest runs in |
…1197) `inspect.signature` and `help()` are built from a text signature: the first line of a C-level callable's docstring, before the `--` separator. That line has to parse as a Python parameter list, and it has to match how the callable is really called. Fourteen of those lines in `_core.c` fail one or the other, and between them they cover sixteen callables. Three do not parse. `msgspec.Raw` declares `Raw(msg="", /)`, which is two adjacent C string literals concatenating to `Raw(msg=, /)`. `msgspec.StructMeta` declares `*, **struct_config`. `msgspec.structs.replace` declares `replace(struct, / **changes)`, missing a comma. For `Raw`, `inspect.signature` raises `ValueError: <class 'msgspec.Raw'> builtin has invalid signature` on every supported version, and the API docs render `class msgspec.Raw` with no parameters. `replace` behaves that way from Python 3.12 onwards: up to 3.11 the stray `/` was stripped along with the rest of the non-Python syntax, leaving something that parsed, and from 3.12 it is kept. `StructMeta` has a `__signature__` getset, so its string only reached `help()` and `docs/api.rst`, which hard-codes the same text. Four more parse, but describe the wrong calling convention. Keyword arguments are rejected at runtime by `msgspec.msgpack.Ext`, `msgspec.structs.asdict`, `msgspec.structs.astuple` and `msgspec.structs.force_setattr`, yet their parameters are marked as positional-or-keyword. The stubs carry the same omission, so `msgspec.structs.asdict(struct=p)` passes type checking and raises `TypeError` when run. The remaining seven begin with a bare `self` rather than CPython's `$self` marker, so the instance becomes a parameter of the bound method: `inspect.signature(encoder.encode)` gives `(self, obj, /)`, and binding a single argument to that fails although `encoder.encode(obj)` is the correct call. Nine bound methods are affected: `encode`, `encode_into` and `encode_lines` on `json.Encoder`, `encode` and `encode_into` on `msgpack.Encoder`, `decode` and `decode_lines` on `json.Decoder`, `decode` on `msgpack.Decoder`, and `Raw.copy`. In `Raw.copy` that `self` carries no trailing `/` either, so it was reported as keyword-addressable on top of being reported at all. Ten stub declarations gain the matching `/`. The unit tests assert the exact reported signature of fifteen callables against a literal, and compile the `StructMeta` text signature. Five negative checks in `tests/typing/basic_typing_examples.py` pin the tightened stubs in that file's existing idiom. `stubtest` couples four of the ten stub edits to the C side by itself; the `Ext.__init__` edit is pinned by nothing else, and the five `self` edits have no observable effect to pin. On current main, fifteen of the sixteen unit cases fail on Python 3.11 and all sixteen on 3.14, and mypy, pyright and pyrefly each flag all five negative checks as unused ignores. Not addressed here: four decoder strings give `type='Any'` as a default, where the real default is `typing.Any` rather than that string. A text signature cannot carry that value. A default written as a name is evaluated, and the result is kept only if it is one of a handful of scalar types, so `type=Any` and `type=typing.Any` alike make the whole signature invalid and `inspect.signature` then raises for the callable. Correcting this one means going outside the text signature. This continues msgspec#1116, which corrected nine such strings. --------- Co-authored-by: Tseluiko Aleksandr <4410812+Siyet@users.noreply.github.com>
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.