Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ChainMap has wrong or unintuitive type after #6042 #8430

Open
kaste opened this issue Jul 28, 2022 · 1 comment
Open

ChainMap has wrong or unintuitive type after #6042 #8430

kaste opened this issue Jul 28, 2022 · 1 comment

Comments

@kaste
Copy link
Contributor

kaste commented Jul 28, 2022

#6042/#6044 gives me new mypy complaints on my code base.

ChainMap only ever writes to the first mapping in maps. That's my understanding. For example ChainMap({}, defaults) is a typical use-case where the user gets a dict-like and cannot mutate the defaults. That's the typical stacking property of the ChainMap, you only ever mutate the top, most recent mapping.

From my understanding:

def __init__(self):
def __init__(self, __map: MutableMapping):
def __init__(self, __map: MutableMapping, *maps: ImmutableMapping):

I don't think we can have correct hints for the maps member, and new_child of course expects a mutable mapping because iirc it is exactly the next top most thing on the stack.

Originally posted by @kaste in #6042 (comment)

(I started this as a comment on the closed issue but closed issues don't move and I got initial supportive feedback on my comment from @hauntsaninja.)

@kaste
Copy link
Contributor Author

kaste commented Jul 28, 2022

@hauntsaninja wrote

That sounds reasonable to me. I would be in favour of:

class ChainMap(MutableMapping[_KT, _VT], Generic[_KT, _VT]):
     maps: list[Mapping[_KT, _VT]]
     def __init__(self) -> None: ...
     def __init__(self, __map: MutableMapping[_KT, _VT], *maps: Mapping[_KT, _VT]) -> None: ...

Also just to call something out:

My understanding is that the typeshed policy is to err on the side of false negatives ... Here is an example in a ChainMap subclass ...

While I'm not sure we've stated this out loud, historically, typeshed has very consistently preferred false positives for subclassers of classes over false positives for other users of classes.

ssbarnea added a commit to ssbarnea/python that referenced this issue Jan 21, 2023
Fixes problems reported by mypy.

Related: python/typeshed#8430
pawamoy pushed a commit to mkdocstrings/python that referenced this issue Jan 23, 2023
Fixes problems reported by mypy.

Related typeshed issue: python/typeshed#8430
PR #53: #53
pawamoy added a commit to mkdocstrings/python that referenced this issue Oct 9, 2023
This was done in commit 3cbe472
to avoid a type warning that the second and next arguments to ChainMap
are not mutable.
Related Mypy issue: python/typeshed#8430.

When we use `!relative` in MkDocs configuration, the config
then contains a MkDocs path placeholder instance, which itself contains
a reference to the MkDocs config, and all its plugins,
including mkdocstrings, and all its objects, etc.

Upon deepcopying this huge object tree, deepcopy fails on
mkdocstrings private attribute `_inv_futures`, which contains
`Future` and therefore `thread.RLock` objects, which are not serializable.
This failed with a `TypeError: cannot pickle '_thread.RLock` objects`.

So in this commit we stop deep-copying everything
just to avoid a Mypy warning, and instead we add a type-ignore comment.
If Mypy fixes this someday, we'll simply get a new warning that the
comment is unused.
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

No branches or pull requests

1 participant