Repository navigation
refactor: replace direct dict subclasses and plain dict data fields with Info or Data #2726
Description
Ruling (owner, in session): do not subclass dict directly. Use Info, or a Data model, wherever one fits, and above all in pcapkit.protocols.data.
On main at 01f063467:
- Direct
dictsubclasses:FrameRecordinfoundation.traceflow.data,_Registriesinconst.reg.apptype, andMultiDictincorekit.multidict.MultiDictis itself a mapping implementation, so whether it is in scope needs a reason either way. - Plain
dictinprotocols.data: the PCAP-NG Decryption Secrets Block'sentriesis typed and built as a plaindictofOrderedMultiInfo. - Also to audit:
read()paths that hand a plaindictinto a data model field, and theTypedDictbit-field shapes inprotocols.schemawherever their values end up stored on a data model.
Convert each one where Info or Data fits, and give the reason for each one left. Pickle and to_dict() compatibility with main must hold. Changing a public type is breaking and needs a versionchanged.
Activity
JarryShaw commentedon Nov 3, 2026
A worker is on this, based on
01f063467. It will audit every directdictsubclass and every plaindictfield inprotocols.data, then convert each one whereInfoorDatafits, with pickle andto_dict()compatibility kept againstmain.wip.JarryShaw commentedon Nov 3, 2026
_RegistriesandMultiDictstay.JarryShaw commentedon Nov 3, 2026
Ruling taken:
_RegistriesandMultiDictstay asdictsubclasses. The conversion is scoped toFrameRecordand to the plaindictfields inprotocols.data, starting with PCAP-NG DSBentries. It also covers anyread()path the audit finds handing a plaindictinto a data model.The worker is already on it, and I've passed this scope to it.
wip.JarryShaw commentedon Nov 3, 2026
Actually
_Registriesshould move as well.JarryShaw commentedon Nov 3, 2026
Ruling taken:
_Registriesis converted as well. OnlyMultiDictstays adictsubclass.How this fits with the rest:
_Registrieslives inpcapkit/const/reg/apptype/__init__.py, the file the new lazy-import work rewrites to load SCTP and DCCP lazily, per your in-session ruling. To avoid two workers clobbering one file, its conversion goes into that change. The new issue tracks it. This issue keepsFrameRecordand the plaindictfields inprotocols.data.JarryShaw commentedon Nov 3, 2026
The worker converted
FrameRecordand the PCAP-NG DSBentries, and a Sonnet review passed it. The PR follows once a small fold lands.Needs deciding:
_EnumRegistry, adefaultdictsubclass inprotocols/schema/schema.py, is the one directdictsubclass left that no ruling covers. It is the mutable class-level registry thatregister()writes into.Infocannot be.InfoorData, for example by holding an immutableInfoand rebuilding it on eachregister().Recommendation: 1. A registry that callers extend at runtime is the kind of genuinely mutable mapping
MultiDictis. Going ahead with 1 unless you pick 2.JarryShaw commentedon Nov 3, 2026
The PR is #2734.
_EnumRegistrystill waits on the decision above.JarryShaw commentedon Nov 3, 2026
okay, take 1
JarryShaw commentedon Nov 3, 2026
Ruling: option 1.
_EnumRegistry(defaultdict)stays as it is: a mutable class registry, where a mapping base is warranted. That was the last open question here. #2734 (dict subclasses and data fields) and #2735 (_Registriesvia #2729) cover the rest, and this closes once both have merged.refactor!: hold mapping values in Info or Data, not dict subclasses (#…
perf(const): import the const enum packages lazily (#2729) (#2735)
JarryShaw commentedon Nov 3, 2026
Closing: #2734 (Info or Data instead of dict subclasses, plus the data-model fields) and #2735 (
_Registriesas anInfo, under #2729) have both merged. The ruling above keeps_EnumRegistry(defaultdict)as it is, so nothing remains open.