Skip to content

mypy doesn't understand set operations on optional #13192

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
Palpatineli opened this issue Jul 19, 2022 · 7 comments · May be fixed by python/typeshed#11403
Closed

mypy doesn't understand set operations on optional #13192

Palpatineli opened this issue Jul 19, 2022 · 7 comments · May be fixed by python/typeshed#11403
Labels

Comments

@Palpatineli
Copy link

I have tried to search for similar issues but failed because I can't find the keywords to limit the open issues to a manageable list. Sorry if it's indeed an old bug. Hope someone here can help

Bug Report

mypy doesn't seem to understand that Set[Optional[T]] minus None is just Set[T]

To Reproduce

from typing import Set
a: Set[str | None] = {'a', None, 'b'}
b: Set[str] = a - {None}
c = ','.join(b)
b2: Set[str] | None = None if None in a else a

Expected Behavior

both line 3 and line 5 should be correct

Actual Behavior

> python3 -m mypy temp.py
temp.py:3: error: Incompatible types in assignment (expression has type "Set[Optional[str]]", variable has type "Set[str]")
temp.py:5: error: Incompatible types in assignment (expression has type "Optional[Set[Optional[str]]]", variable has type "Optional[Set[str]]")
Found 2 errors in 1 file (checked 1 source file)

Your Environment

  • Mypy version used: 0.971
  • Mypy command-line flags: none
  • Python version used: 3.10.4
  • Operating system and version: Ubuntu 22.04LTS
@Palpatineli Palpatineli added the bug mypy got something wrong label Jul 19, 2022
@AlexWaygood
Copy link
Member

AlexWaygood commented Jul 20, 2022

The typeshed stub for the __sub__ method of builtins.set is currently:

from typing import MutableSet, Generic, AbstractSet, TypeVar

_T = TypeVar("_T")

class set(MutableSet[_T], Generic[_T]):
    def __sub__(self, __s: AbstractSet[_T | None]) -> set[_T]: ...

I guess we could special-case None in the stub:

from typing import MutableSet, Generic, AbstractSet, TypeVar, overload

_S = TypeVar("_S")
_T = TypeVar("_T")

class set(MutableSet[_T], Generic[_T]):
    @overload
    def __sub__(self: set[_S | None], __s: AbstractSet[None]) -> set[_S]: ...
    @overload
    def __sub__(self, __s: AbstractSet[_T | None]) -> set[_T]: ...

But I'm not sure this kind of thing is common enough to be worth special-casing here.

Such a change would have to happen over at the typeshed repo rather than the mypy repo, since mypy vendors its stdlib stubs from typeshed.

@AlexWaygood AlexWaygood added feature and removed bug mypy got something wrong labels Jul 20, 2022
@tungol
Copy link
Contributor

tungol commented Jul 22, 2022

Shouldn't the special casing should really be for singleton types in general, similar to the changes made here? #13117

@TeamSpen210
Copy link
Contributor

Well that one is different since it's about is and if - core syntax, unlike set operations here which are regular magic method operations.

Implementing the other singleton objects too would be tricky, since you'd need to account for all possible combinations of NotImplemented, None, Ellipsis, True and False. Bools might not really work since {False, True} is just going to have a type of set[bool], it's not going to track whether or not both values are present which you'd need in order to remove from the union.

@AlexWaygood
Copy link
Member

I agree with @TeamSpen210. There's no generalised way in the type system at the moment of indicating that a type is a singleton, and I don't think there would be enough use cases to make it worth the added complexity of creating a generalised way of doing that. The changes made in #13117 are highly specific to EllipsisType; it's the opposite of "special casing for singleton types in general".

I'm closing this as I don't think there's anything actionable for mypy here. If we did want to do something in this area, the change would have to be made over at typeshed.

@AlexWaygood AlexWaygood closed this as not planned Won't fix, can't repro, duplicate, stale Jul 23, 2022
@finite-state-machine
Copy link

finite-state-machine commented Feb 11, 2024

Is there a suggested workaround, short of resorting to a cast()?

I've tried the kitchen sink without success (mypy-play.net):

from __future__ import annotations
from typing import *

base_chars: Set[Optional[str]]
chars = frozenset(base_chars - {None})
assert None not in chars
assert all(chars)
assert all(c is not None for c in chars)
assert all(isinstance(c, str) for c in chars)
reveal_type(chars)   # FrozenSet[Union[str, None]]

[edited to use frozenset() rather than set()]

@erictraut
Copy link

Here's a straightforward workaround:

def func(base_chars: set[str | None]):
    chars = frozenset(x for x in base_chars if x is not None)
    reveal_type(chars) # frozenset[str]

@finite-state-machine
Copy link

finite-state-machine commented Feb 11, 2024

Thanks, @erictraut! (I hadn't considered a comprehension because it wasn't verifying the type of the set, but that works 99% of the time. Where zero-copy/performance really matters, there's always cast().)

If this issue is ever reconsidered, I'd like to offer a thought: perhaps an every() function would help make set narrowing more type-agnostic and relatively affordable to implement:

Te = TypeVar('Te', bound=bool|Enum|Ellipsis|None)

def every(type_: Type[Te], /) -> CompleteFrozenSet[Te]:
    '''returns every member of an enumerable type
    
    Args:
        type_: 'None', 'bool', 'Ellipsis', or an 'Enum' type
    Returns:
        a frozen set containing every value of the given type
    '''
    # magic happens
    return ...
    

class CompleteFrozenSet(FrozenSet[T]):
    '''(empty body)'''
    

# (I'm not sure if this would work as written:)
class Set(...):
    ...
    @overload
    def __isub__(self: Set[A|B], other: CompleteFrozenSet[B]) -> Set[A]: ...
    ....

I suppose an def __every__(self: Type[Te]) -> CompleteFrozenSet[Te]: ... magic method would be the natural extension to other types.

I do realize this probably isn't a priority.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants