Skip to content

Conversation

@jonathangreen
Copy link
Contributor

Summary

This PR fixes mypy type errors that occur when passing simple dictionaries to URI template expansion methods. The core issue is that Dict is invariant in its value type, while Mapping is covariant, allowing for more flexible type checking.

Problem

When users try to expand templates with a simple dictionary, mypy raises type errors:

params: dict[str, str] = {'param1': 'abc', 'param2': 'xyz'}
template.expand(params)

Mypy error:

error: Argument 1 to "expand" of "URITemplate" has incompatible type "dict[str, str]"; 
       expected "dict[str, Sequence[int | float | complex | str | None] | list[int | float | complex | str | None] | 
       Mapping[str, int | float | complex | str | None] | tuple[str, int | float | complex | str | None] | 
       int | float | complex | str | None] | None"  [arg-type]
note: "dict" is invariant -- see https://mypy.readthedocs.io/en/stable/common_issues.html#variance
note: Consider using "Mapping" instead, which is covariant in the value type

Why does this happen?

The issue stems from type variance:

  • Dict[str, VariableValue] is invariant: A dict[str, str] is NOT compatible with Dict[str, VariableValue], even though str is part of the VariableValue union
  • Mapping[str, VariableValue] is covariant: A Mapping[str, str] IS compatible with Mapping[str, VariableValue] because mappings are read-only

Since the expansion functions only read from the dictionary (they don't mutate it), Mapping is the correct type annotation.

Changes Made

1. Renamed type alias to better reflect immutability

# Before
VariableValueDict = t.Dict[str, VariableValue]

# After  
VariableValueMapping = t.Mapping[str, VariableValue]

2. Fixed the _merge() function

The Mapping type doesn't have a .copy() method, so we use the dict() constructor instead:

# Before
def _merge(...) -> VariableValueDict:
    if var_dict:
        opts = var_dict.copy()  # Error: Mapping has no attribute 'copy'
        
# After
def _merge(...) -> VariableValueMapping:
    if var_dict:
        opts = dict(var_dict)  # Works with any Mapping

3. Updated all type annotations

  • uritemplate/variable.py: Updated type alias definition
  • uritemplate/template.py: Updated _merge(), _expand(), expand(), partial()
  • uritemplate/api.py: Updated expand() and partial() functions
  • tests/: Updated all test type annotations for consistency

Benefits

  1. Fixes user-facing mypy errors: Users can now pass dict[str, str] without type errors
  2. Better type semantics: Mapping better expresses that the functions don't mutate inputs
  3. More flexible: Accepts any mapping-like object (dict, OrderedDict, ChainMap, etc.)
  4. Backwards compatible: dict is a subtype of Mapping, so all existing code works

Testing

  • All pre-commit hooks pass (including mypy)
  • All existing tests pass
  • No behavioral changes - only type annotations
  • Verified that dict[str, str] can now be passed to expand() without mypy errors

References

Changed VariableValueDict to VariableValueMapping and updated type
from Dict to Mapping to properly represent immutable type annotations.
Fixed _merge function to use dict() constructor instead of .copy()
since Mapping types don't have a copy method.
@sigmavirus24 sigmavirus24 merged commit 4994ed2 into python-hyper:main Oct 4, 2025
8 checks passed
@jonathangreen jonathangreen deleted the fix-mypy-type-errors branch October 4, 2025 01:13
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.

2 participants