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

Improve documentation for decisions around certain list, dict, Mapping, and Sequence methods #7015

Open
ktbarrett opened this issue Jan 24, 2022 · 6 comments
Labels
project: infrastructure typeshed build, test, documentation, or distribution related

Comments

@ktbarrett
Copy link
Contributor

list.index and list.count currently only accepts values of the element type and Sequence.index and Sequence.count use Any for the type of the argument. In my opinion all of these functions could safely take object.

dict.get and Mapping.get currently only accepts values of the key type. This could be safely extended to be any Hashable object. Non-Hashable values will raise TypeError, but runtime safely accepts values of any other type. I ran into someone on the gitter who needed this a few weeks ago, but he never opened an issue.

I would be willing to make a PR for this.

@JelleZijlstra
Copy link
Member

It's true that these functions will take any argument at runtime and return a result, but I'm not convinced that that's what we should be doing in the stub. list[int].index("str") may work, but it's almost certainly not what the user wants, so I'd like type checkers to be able to tell that this code is suspicious.

@hauntsaninja
Copy link
Collaborator

These kinds of methods are typeshed top hits. E.g., just last week: #6922
I think it'd be helpful to document these decisions, at least as comments in the stubs.

@ktbarrett
Copy link
Contributor Author

This diverges what is valid according to typing and what is valid at runtime in core libraries, which makes annotating existing code bases that might utilize this behavior correctly to be considered incorrect. I didn't know it was in typing's scope to decide that certain core Python behavior isn't valid, are you sure that isn't overreaching?

@srittau
Copy link
Collaborator

srittau commented Jan 24, 2022

typeshed's purpose is to help developers to find mistakes in their code. As @JelleZijlstra pointed out, these methods are typed to find likely problems, not necessarily to accept everything that doesn't crash at runtime.

@Akuli
Copy link
Collaborator

Akuli commented Jan 24, 2022

Ideally we would have a way to say "this method accepts any type that overlaps with this type", so you could do list_of_strings.index(string_or_none) or list_of_strings_or_nones.index(string), but not list_of_ints.index(string).

@ktbarrett
Copy link
Contributor Author

All of the examples I could come up with (see one below) would be serviced by a Super, denoting that any supertype is accepted, not subtype. Something that checks for Union overlapping would be even less strict, but would have very limited use cases otherwise. Something like Super could be used in many cases.

This is just an stupid example of a pattern I have used and seen others use before. You handle some cases using a dict and .get() and if .get() returns the sentinel you continue on and do something else. In these cases the type of the argument is usually a supertype/superset of the type of the dict.

def stringify(value: object) -> str:
    # handle a few of the values first
    a = ({None: "None", True: "True", False: "False", ...: "..."}).get(value)
    if a is not None:
        return a
    # rest are handled by arbitrary logic
    ...

@AlexWaygood AlexWaygood changed the title Some list, dict, Mapping, and Sequence methods are unnecessarily strict Improve documentation for decisions around certain list, dict, Mapping, and Sequence methods Jun 29, 2022
@srittau srittau added project: infrastructure typeshed build, test, documentation, or distribution related and removed docs labels Mar 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project: infrastructure typeshed build, test, documentation, or distribution related
Projects
None yet
Development

No branches or pull requests

6 participants