Migrate all modules off docval + real signatures for generated classes (Phases 3-4 of #1129)#1527
Draft
bendichter wants to merge 5 commits into
Draft
Migrate all modules off docval + real signatures for generated classes (Phases 3-4 of #1129)#1527bendichter wants to merge 5 commits into
bendichter wants to merge 5 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## typing-compat-layer #1527 +/- ##
========================================================
- Coverage 91.79% 38.09% -53.70%
========================================================
Files 50 51 +1
Lines 11027 10952 -75
Branches 2302 2337 +35
========================================================
- Hits 10122 4172 -5950
- Misses 558 6365 +5807
- Partials 347 415 +68 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
bendichter
force-pushed
the
typing-phase3-core
branch
from
July 4, 2026 16:23
cae43d7 to
eff3bae
Compare
bendichter
changed the title
Migrates all @DocVal sites in common/table.py (VectorData, VectorIndex, ElementIdentifiers, DynamicTable, DynamicTableRegion, EnumData, MeaningsTable), common/alignedtable.py, and common/multi.py to type-hinted signatures with @validated. The three AlignedDynamicTable methods that splice parent specs via @DocVal(*get_docval(...)) intentionally stay on @DocVal and now consume synthesized specs from the migrated DynamicTable parents. Hand-work beyond the tool: mutable list() defaults -> None-guards (VectorData.data and friends), super().__init__(**kwargs) -> explicit keywords, and the allow_extra kwargs-meaning change (docval's kwargs held ALL parsed args; the real **kwargs holds only extras) in add_column, from_dataframe, and MeaningsTable.add_row. The migrate tool now flags kwargs uses in allow_extra bodies with a dedicated warning. Verified: full hdmf suite green (1985); full pynwb dev suite green (804 tests + 6581 subtests) against this branch — DynamicTable and VectorData are pynwb's most-subclassed classes and its get_docval splices now run entirely on synthesized specs for these types. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Migrates all @DocVal sites in spec/spec.py, spec/namespace.py, spec/catalog.py, and spec/write.py to type-hinted signatures with @validated. The shared module-level spec-argument lists (_attr_args, _group_args, _dataset_args, ...) are inlined into signatures and removed; the migrate tool learned to resolve such module-level literal list splices automatically (36/39 spec.py sites auto-converted). Hand-work: the three _target_type_key sites (RefSpec, LinkSpec, GroupSpec.add_link), explicit super()/constructor forwarding in DatasetSpec/GroupSpec and the deprecated add_* wrappers, list() default guards, and within-module forward refs as whole-string annotations ("BaseStorageSpec | None") instead of TypeName, which static checkers can resolve. Behavior note: missing/extra-argument errors on migrated functions use Python's native message wording; one hdmf test updated accordingly. Runtime spec introspection (Spec.build_spec, build_namespace) is unchanged — it reads get_docval(cls.__init__), served by the shim. Verified: full hdmf suite green (1985); full pynwb dev suite green (804 + 6581 subtests) — pynwb/spec.py builds its NWB spec classes by introspecting these constructors via get_docval. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bendichter
force-pushed
the
typing-phase3-core
branch
from
July 4, 2026 16:34
8c866c0 to
40a1d5f
Compare
Migrates the 11 static @DocVal sites in container.py (AbstractContainer, Container, Data, DataRegion, Table.__init__ and methods) to type-hinted signatures with @validated. The 8 dynamically composed decorators (the MultiContainerInterface __make_* factories and the Row/Table row-class machinery, which compute docs/types/func_name at class-generation time) intentionally stay on @DocVal and are annotated as such; they migrate with docval removal (#1129 Phase 4). The AbstractContainer metaclass field-docs machinery (get_docval(cls.__init__) at class definition) now reads synthesized specs for migrated classes. Verified: full hdmf suite green (1985); full pynwb dev suite green (804 + 6581 subtests) — every pynwb container class roots its get_docval splice chain in these constructors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… 4-5) Migrates all remaining static @DocVal sites: build/builders.py, build/manager.py, build/objectmapper.py, build/errors.py, backends/io.py, backends/utils.py, backends/hdf5/h5_utils.py (incl. H5DataIO), backends/hdf5/h5tools.py (incl. HDF5IO write/export/export_io), common/__init__.py, common/io/table.py, validate/errors.py, and term_set.py. Remaining @DocVal uses are exclusively: the spec-splicing composition sites (@DocVal(*get_docval(...)), annotated as intentional, consuming synthesized specs) and build/classgenerator.py (generates constructors for schema-derived classes; migrates in Phase 4). Notable hand-work: H5DataIO.__init__'s kwargs-partitioning body rebuilt around named parameters; HDF5IO.export/export_io given real signatures (the class-level _export_args tuple is gone); write/write_builder/ write_group forwarding dicts rebuilt from named params. Reverted the whole-string forward-ref annotations introduced in the spec batch back to TypeName[...] — string annotations naming later-defined classes do not resolve at decoration time; hdmf.typing now warns if a compound string annotation cannot be resolved instead of silently never matching. Verified: full hdmf suite green (1985); docs build zero-warning; full pynwb dev suite green (804 + 6581 subtests). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bendichter
changed the title
Adds hdmf.typing.signature_function, a runtime builder that converts docval-style argument spec dicts into functions with real type-hinted signatures (exec-built like dataclasses), Google-style docstrings, and @validated runtime checking. Mutable defaults keep docval's per-call deepcopy semantics; non-identifier argument names (exercised by tests) fall back to legacy @DocVal. Used everywhere functions are composed at runtime: - build/classgenerator.py: schema-generated __init__ (both CustomClassGenerator and MCIClassGenerator) — get_class() classes now show real constructor signatures in IDEs and help() - container.py MultiContainerInterface __make_get/__make_getitem/ __make_add/__make_create/__make_constructor - container.py Row/Table dynamic __init__ and add_row With this, the only remaining @DocVal decorators in hdmf are the eight @DocVal(*get_docval(...)) splice sites, which migrate with docval removal. Also adds the py.typed marker. Test updates: docval's synthetic signature line is gone from generated docstrings (real signatures replace it); 'bool' string vs bool class in one generated spec (validates identically); native missing-argument message wording. Verified: full hdmf suite green (1994); docs zero-warning; full pynwb dev suite green (804 + 6581 subtests) — pynwb's extension classes are generated through this exact machinery. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
bendichter
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #1526. Migrates the highest-risk modules for downstream compatibility — the classes PyNWB subclasses and splices most heavily:
common/table.py:VectorData,VectorIndex,ElementIdentifiers,DynamicTable,DynamicTableRegion,EnumData,MeaningsTable— all 22@docvalsites now type-hinted signatures with@validatedcommon/alignedtable.py: all literal sites migrated; the three methods that splice parent specs via@docval(*get_docval(DynamicTable...))intentionally remain on@docvaland consume specs synthesized from the migrated parentscommon/multi.py:SimpleMultiContainerNotable hand-work (patterns for reviewers to check):
list()defaults →= None+ body guard (VectorData.dataetc.); synthesized specs reportdefault: None, and constructors behave identically when the arg is omittedallow_extrafunctions: docval'skwargsheld ALL parsed args, real**kwargsholds only extras —add_column's column-constructor kwargs,from_dataframe, andMeaningsTable.add_rowwere rewritten accordingly (the migrate tool now flags this pattern)Verification: full hdmf suite green (1985); full PyNWB dev suite green (804 tests + 6581 subtests) against this branch.
Remaining Phase 3 (follow-ups):
container.py,spec/,build/,backends/hdf5/.🤖 Generated with Claude Code