Skip to content

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

Closed
KotlinIsland wants to merge 1 commit into
python:masterfrom
KotlinIsland:fix_NoneType
Closed

KotlinIsland wants to merge 1 commit into
python:masterfrom
KotlinIsland:fix_NoneType

Conversation

@KotlinIsland

@KotlinIsland KotlinIsland commented Oct 7, 2021 •

Copy link
Copy Markdown
Contributor

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

github-actions Bot commented Oct 7, 2021

Copy link
Copy Markdown
Contributor

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

@erictraut

Copy link
Copy Markdown
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

KotlinIsland commented Oct 7, 2021 •

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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
Copy Markdown
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

KotlinIsland commented Oct 7, 2021 •

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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
Copy Markdown
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

KotlinIsland commented Oct 7, 2021 •

Copy link
Copy Markdown
Contributor Author

when NoneType is used

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

@KotlinIsland

Copy link
Copy Markdown
Contributor Author

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

@erictraut

Copy link
Copy Markdown
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

KotlinIsland commented Oct 7, 2021 •

Copy link
Copy Markdown
Contributor Author

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