Make Multitext an actual Mapping, breaking keys(), len(), and duplicate langs - #42
Draft
imnasnainaec wants to merge 2 commits into
Draft
Make Multitext an actual Mapping, breaking keys(), len(), and duplicate langs#42imnasnainaec wants to merge 2 commits into
imnasnainaec wants to merge 2 commits into
Conversation
The class hand-wrote get, __contains__, keys, values and items on top of __getitem__ / __iter__ / __len__ — which is the set collections.abc.Mapping derives from those three. Inheriting it drops them, and makes isinstance(mt, Mapping) true for consumers that ask. keys(), values() and items() therefore return views rather than lists. A view is what a Mapping promises, and typing the class as one while returning lists would have been three suppressed Liskov violations; set operations on keys() work now, and a caller wanting a list can say so. __iter__ and __len__ read forms directly, since the views are built on them. __len__ counts languages rather than forms. It counted every form including a lang=None one, which keys() has always excluded, so len(mt) could exceed len(mt.keys()) on schema-invalid input; as a Mapping that would leave len(mt) != len(list(mt)). __bool__ still answers "is there anything to serialize", which residue and a lang-less form each defeat on their own, so it stays independent of len(). The two mutators stay as they are. MutableMapping is not inherited: clear and popitem have no clear meaning for a form list that can hold forms no key reaches. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A language repeated across forms was yielded once per form, so the inherited views walked it twice and resolved both to the first form's text: values() reported that text twice and never the second form's, and len() counted 2 where dict() held 1 key. A repeated language is exactly the schema-invalid input validation reports as duplicate-form-lang, so it is real FLEx and WeSay output rather than a hypothetical. __iter__ now yields each language once — the one __getitem__ answers with — and __len__ counts those, so len(mt) == len(dict(mt)) whatever the forms hold. Nothing is hidden: forms still holds every form, which is where duplicate-form-lang reads from and where a lang-less form was already the docstring's example of content no mapping can represent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
imnasnainaec
added a commit
that referenced
this pull request
Aug 26, 2026
Inheriting collections.abc.Mapping changes three things callers can see: keys(), values() and items() return views rather than lists, len() counts languages rather than forms, and a language spelled on two forms becomes one key. Accepting that is a judgement about how much of a 0.x API is worth breaking for a correct protocol, which nothing here shares a file with — the rest of this branch deletes duplicated code without changing what anything returns. It is proposed on its own in #42, so it can be taken or refused without holding up six changes that are only refactors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Multitexthand-wroteget,__contains__,keys,valuesanditemsontop of
__getitem__/__iter__/__len__— which is the setcollections.abc.Mappingderives from those three. Inheriting it drops them,and makes
isinstance(mt, Mapping)true for consumers that ask.This breaks three things, which is the whole decision:
keys(),values(),items()len(mt)Each has a reason to be the way it now is:
Views. Typing the class as a
Mappingwhile returning lists means threesuppressed Liskov violations — a
listis not aKeysView, and a callertyped against
Mapping[str, Text]would be promised set operations it has notgot. Set operations on
keys()work now, and a caller wanting a list can sayso. It cost one assertion in the suite, which mypy pointed at; nothing in
docs/relied on list semantics, since the guides only ever show[...],in, and assignment.Counting languages.
len()counted every form including a lang-less one,which
keys()has always excluded, solen(mt)could already exceedlen(mt.keys())on schema-invalid input. As aMappingthat would leavelen(mt) != len(list(mt)).One key per language. A repeated language was yielded once per form, so the
views resolved every one of them to the first form's text:
values()reportedthat text twice and never the second form's, and
len()disagreed withlen(dict(mt)). A repeated language is exactly what validation reports asduplicate-form-lang— real FLEx and WeSay output, not a hypothetical.Nothing is hidden by any of it:
formsstill holds every form, and is whereduplicate-form-langreads from.MutableMappingis deliberately not inherited.clearandpopitemhave noclear meaning for a form list that can hold forms no key reaches, so the two
mutators the class already had are still the only two.
python scripts/check.pygreen: 573 passed.🤖 Generated with Claude Code
This change is