Skip to content

Expand read-only Mapping values as associative arrays - #167

Open
anishpentyala wants to merge 2 commits into
python-hyper:mainfrom
anishpentyala:fix/read-only-mapping-values
Open

anishpentyala wants to merge 2 commits into
python-hyper:mainfrom
anishpentyala:fix/read-only-mapping-values

Conversation

@anishpentyala

Copy link
Copy Markdown

Problem

Nested read-only mappings satisfy VariableValue's existing Mapping[str, ScalarVariableValue] annotation, but runtime detection only recognizes dict and MutableMapping. As a result, a MappingProxyType or a custom read-only Mapping is expanded as its quoted Python representation rather than as an associative value.

For example:

from types import MappingProxyType
from uritemplate import expand

expand("{?v*}", v=MappingProxyType({"a": "one", "b": "two"}))
# Expected: ?a=one&b=two

Change

Use collections.abc.Mapping in dict_test. The existing associative expansion paths only read the mapping, so they do not need the stronger mutable interface.

The regression tests cover standard MappingProxyType and a custom read-only Mapping across all eight operators, with and without explode. They also check sorted-key behavior, no mutation, quoted-key/value parity with dictionaries, and that scalar/list/list-of-pairs values are not reclassified.

This follows the outer-mapping typing work in #130, but fixes nested associative value detection at runtime. It does not change the existing empty-collection behavior discussed in #164 or attempt to fix unrelated prefix/escaping behavior.

Validation

  • Existing baseline: 58 passed, 66 subtests passed.
  • The 53 added cases against unchanged runtime: 48 failed, 5 controls passed.
  • Full suite after the fix: 111 passed, 66 subtests passed on Python 3.10.12 and 3.13.17.
  • Fresh clone of the public branch: exact reviewed source tree and 111 passed, 66 subtests passed on Python 3.13.17.
  • Black, isort, flake8 and strict mypy over all eight source files passed locally.

Hosted checks remain separate from these local results.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant