You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Currently NestedMultiDict inherits from MultiDict even though it is clearly not a MutableMapping. I understand the convenience of being able to both accept MultiDict and NestedMultiDict with a simple isinstance(x, MultiDict) check, so I propose to add a couple of abstract base classes, namely MultiMapping and MutableMultiMapping which inherit from Mapping and MultiMapping, MutableMapping respectively.
That way you can still accept any immutable MultiDict via MultiMapping but cleanly rule out NestedMultiDict for a function which requires a MutableMultiMapping (or MutableMapping). This is especially relevant for static type checking. At runtime you do emit exceptions with the current implementation, but it will generally end up being deeper in the call-stack than you would prefer. Also the current implementation circumvents hasattr checks.
This would be a breaking change, but I think it is worth it to improve the situation with type checkers. types-WebOb provides good type hints for WebOb but it can't get around this inheritance problem, so NestedMultiMapping can still be passed into functions which it shouldn't be passed into. The type stubs could lie and pretend NestedMultiMapping does not inherit from MultiDict, but that would break the opposite case, where you didn't care about mutability and adding fake types that people need to import inside if TYPE_CHECKING: blocks is bad form.
multidict which is used in aiohttp chose this exact approach (they have abstract MultiMapping and MutableMultiMapping classes), so you could also just add it as a new dependency and reuse their implementation and extend it with NoVars and NestedMultiDict (which should inherit from MultiMapping) to keep things backwards compatible. The advantage of that approach would be that multidict has a C extension, so it should be significantly faster than this pure-python implementation.
The text was updated successfully, but these errors were encountered:
Currently
NestedMultiDict
inherits fromMultiDict
even though it is clearly not aMutableMapping
. I understand the convenience of being able to both acceptMultiDict
andNestedMultiDict
with a simpleisinstance(x, MultiDict)
check, so I propose to add a couple of abstract base classes, namelyMultiMapping
andMutableMultiMapping
which inherit fromMapping
andMultiMapping, MutableMapping
respectively.That way you can still accept any immutable
MultiDict
viaMultiMapping
but cleanly rule outNestedMultiDict
for a function which requires aMutableMultiMapping
(orMutableMapping
). This is especially relevant for static type checking. At runtime you do emit exceptions with the current implementation, but it will generally end up being deeper in the call-stack than you would prefer. Also the current implementation circumventshasattr
checks.This would be a breaking change, but I think it is worth it to improve the situation with type checkers.
types-WebOb
provides good type hints forWebOb
but it can't get around this inheritance problem, soNestedMultiMapping
can still be passed into functions which it shouldn't be passed into. The type stubs could lie and pretendNestedMultiMapping
does not inherit fromMultiDict
, but that would break the opposite case, where you didn't care about mutability and adding fake types that people need to import insideif TYPE_CHECKING:
blocks is bad form.multidict
which is used inaiohttp
chose this exact approach (they have abstractMultiMapping
andMutableMultiMapping
classes), so you could also just add it as a new dependency and reuse their implementation and extend it withNoVars
andNestedMultiDict
(which should inherit fromMultiMapping
) to keep things backwards compatible. The advantage of that approach would be thatmultidict
has a C extension, so it should be significantly faster than this pure-python implementation.The text was updated successfully, but these errors were encountered: