Skip to content

Fix config_store map_annotation on Python 3.14 (read-only __args__)#10726

Merged
akihironitta merged 3 commits into
pyg-team:masterfrom
adi-kumo:py314-config-store-fix
Jun 12, 2026
Merged

Fix config_store map_annotation on Python 3.14 (read-only __args__)#10726
akihironitta merged 3 commits into
pyg-team:masterfrom
adi-kumo:py314-config-store-fix

Conversation

@adi-kumo

Copy link
Copy Markdown
Contributor

On Python 3.14, config_store.map_annotation raises AttributeError: readonly attribute: it assigns to __args__, which became read-only after the typing.Union unification (CPython gh-105499). This rebuilds the annotation instead of mutating it.

Relationship to #10692

Same root cause as #10692 — thanks @ddelgadovargas-cyber. Opening this as an alternative because it:

  1. Preserves the typing.* form — uses copy_with(...) so typing.List[...] stays typing.List[...] (rather than normalizing to list[...]), leaving the existing test_map_annotation assertions unchanged.
  2. Also fixes a latent bug on 3.10–3.13: the original copy.copy(alias); alias.__args__ = args doesn't actually copy — copy.copy() on a typing alias returns the same interned object — so remapping one annotation mutates the shared global singleton (e.g. typing.List[int] becomes typing.List[Any] process-wide, and a later Union remap can crash). Rebuilding returns fresh objects and eliminates this.

Happy to consolidate into whichever approach maintainers prefer.

Changes

  • Union/OptionalUnion[args]; other typing.* generics → annotation.copy_with(tuple(args)) (legacy in-place path kept as a fallback); builtin list[...]/dict[...] path unchanged.
  • Added test_map_annotation cases for Union/Optional/nested generics + Union-collapse-to-Any.

Validation

test/test_config_store.py and fill_config_store() pass on Python 3.10 and 3.14.

(Draft — opening for discussion / alignment with #10692.)

adi-kumo and others added 2 commits June 9, 2026 05:34
map_annotation rebuilds Union/list/dict/tuple generics after remapping
their inner types (e.g. torch.Tensor -> Any). For typing aliases it did
`copy.copy(annotation); annotation.__args__ = args`.

Python 3.14 unified typing.Union with the builtin X | Y union type
(types.UnionType) -- they are now the same C-implemented class whose
__args__ is a read-only slot, so the in-place assignment raises
`AttributeError: readonly attribute` at import/registration time for any
class registered via @register whose __init__ has a Union/Optional
annotation. (CPython gh-105499; What's New in Python 3.14.)

Rebuild without mutation instead:
- Union/Optional -> Union[tuple(args)]
- other typing aliases -> annotation.copy_with(tuple(args)), with the
  legacy in-place rewrite kept only as a fallback.

Behavior-preserving on Python 3.10-3.13; unblocks 3.14. Adds Union/Optional
regression assertions to test_map_annotation.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Locks in the one behavioral improvement of the 3.14 __args__ fix: when
all Union members map to the same type, the rebuilt annotation collapses
to Any (old in-place mutation left a degenerate Union[Any, Any]).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@akihironitta akihironitta added the ci: full Run the full test suite (pytest-full) on this PR label Jun 10, 2026
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 71.42857% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 79.68%. Comparing base (c211214) to head (746828b).
⚠️ Report is 214 commits behind head on master.

Files with missing lines Patch % Lines
torch_geometric/config_store.py 71.42% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master   #10726      +/-   ##
==========================================
- Coverage   86.11%   79.68%   -6.43%     
==========================================
  Files         496      513      +17     
  Lines       33655    37870    +4215     
==========================================
+ Hits        28981    30177    +1196     
- Misses       4674     7693    +3019     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@adi-kumo adi-kumo marked this pull request as ready for review June 10, 2026 23:25
@adi-kumo

Copy link
Copy Markdown
Contributor Author

Ready for review. Quick note on CI: the pytest-full reds are pre-existing on master (the scheduled full run fails the same 8 jit.script tests at this PR's base commit 286a2aa) — unrelated to this change; the regular pytest lane is green. Validated on Python 3.10–3.12 and 3.14.

@adi-kumo

Copy link
Copy Markdown
Contributor Author

Thanks for the review @akihironitta! The only thing left is the required checks blocking merge — could you admin-merge / override when you get a chance, or let me know if there's a path you'd prefer?

@akihironitta akihironitta merged commit be5e445 into pyg-team:master Jun 12, 2026
10 of 26 checks passed
@adi-kumo adi-kumo deleted the py314-config-store-fix branch June 12, 2026 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci: full Run the full test suite (pytest-full) on this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants