Skip to content

Make types.NoneType actually the type of None, instead of just some unrelated class #6125

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

Closed
wants to merge 1 commit into from

Conversation

KotlinIsland
Copy link
Contributor

@KotlinIsland KotlinIsland commented Oct 7, 2021

We lose the types from NoneType.__bool__() but I think it's worth it.

@github-actions

This comment has been minimized.

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Oct 7, 2021

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@erictraut
Copy link
Contributor

I don't think this change should be made. It will break assumptions in mypy and pyright (and probably other type checkers) about how _typeshed.NoneType defines the valid methods for the class that is instantiated to create the None object. For example, the following statement should type check, but it will fail if this change is made.

a: Hashable = None

What problem is this PR attempting to solve? If the intent is to enable users to use types.NoneType within type annotations, I think that's a misguided goal. Symbols within the types module are not meant to be used in type annotations within typical code. We regularly need to explain in mypy and pyright issue trackers not to use FunctionType, CoroutineType, etc. repeatedly. This change will lead to further confusion.

@KotlinIsland
Copy link
Contributor Author

KotlinIsland commented Oct 7, 2021

Well I would think that if the type in types is the actual type of something it's typing should reflect that, regardless of what the idioms dictate.

What about something more like:
builtins.pyi:

class _NoneType:
    def __bool__(self) -> Literal[False]: ...
    def __hash__(self) -> int: ...
    
None: _NoneType

types.pyi:

    from builtins import _NoneType
    NoneType = _NoneType

@erictraut
Copy link
Contributor

I don't think that modifications solves anything, and it will break even more than the previous PR.

I would think that if the type in types is the actual type of something it's typing should reflect that

That's not consistent with my understanding. If you're importing types in typical Python code, you're probably doing something wrong. There's already confusion about types and typing. Let's not add to that confusion.

I think this PR is attempting to solve a non-problem. Let's keep things the way they are. If you have a need to refer to the type of None, just use type[None]. It doesn't come up that often anyway.

@KotlinIsland
Copy link
Contributor Author

I guess I just want it for completeness sake.

It would actually be better then if the type checkers disallowed the usage of NoneType as an annotation and recommended the usage of type[None] instead.

@KotlinIsland
Copy link
Contributor Author

KotlinIsland commented Oct 7, 2021

What a mess, I would prefer if NoneType was typed correctly as the type of None, it would make everything just a little more consistent, instead of having this very strange outlier that None is secretly a type alias for Literal[None] and that a
combination of NoneType(or type(None)) and type[None] should be used for typing edge cases.

def foo(n: NoneType):
    isinstance(n, NoneType)
    # consistent? whaaa?

Fair enough though if you guys think this not a good one.

@KotlinIsland
Copy link
Contributor Author

@erictraut @JelleZijlstra What about typing FunctionType and MethodType as Callable? or do you think that will just add confusion on which one to use?

@erictraut
Copy link
Contributor

The classes defined in types do have a reason to exist. They're just not useful for typical Python code. They should be left as is.

I saw that you filed a feature request in the mypy issue tracker to issue an error when NoneType is used. I don't think that's the right answer either because there are legitimate uses for this type. Just not for most people.

I think the best solution here is to improve the documentation to make it clear that most Python code should not import from the types module. It should be used only if you really understand what you're doing and only in very specific circumstances.

@KotlinIsland
Copy link
Contributor Author

KotlinIsland commented Oct 7, 2021

when NoneType is used

I mean used as a type annotation, I can't think of any possible usage.

@KotlinIsland
Copy link
Contributor Author

python/mypy#11285 (comment)
TracebackType is used.

@erictraut
Copy link
Contributor

They're used in tools that need to inspect Python internals. For example, the mypy source code needs to use these types. In these contexts, they're legitimate to use within type annotations.

@KotlinIsland
Copy link
Contributor Author

KotlinIsland commented Oct 7, 2021

I understand that there are always edge cases, and disabling those warnings with a type comment would be the way to go IMO. With the special case of None being a secret alias to Literal[None] I think that many many people are going to try to use NoneType and a warning for the 99% of the time that it's invalidly used would be a great one.

@KotlinIsland KotlinIsland deleted the fix_NoneType branch October 7, 2021 04:22
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