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 |
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.