-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Assignment of Any does not get rid of Optional (add AnyUnion
type?)
#3526
Comments
But doesn't #3501 (or similar) provides a better solution?
Do I understand correctly that this has been solved by PEP 526? In general, I don't think that we really need this feature. What worries me is that in addition to making type system more complex it also can break transitivity of subtyping (or maybe better call this compatibility). IIUC your proposal, then |
Probably it was only partially fixed by PEP 526. But for remaining cases one can just do something like: UNINITIALIZED: Any = None
class C:
def __init__(self) -> None:
self.complicated: int = UNINITIALIZED
# existing code below
while self.complicated is None:
... |
I'm not sure this is needed all that often, but if we do, we could just spell it Regardless I'm not sure that in your original motivating example we should do anything different. If you want to signal at the call site that def g(x: Optional[int]) -> int
if x is None:
y = f() # type: int
x = y
return x # OK |
This is probably the same feature as 'unsafe unions' that have been discussed before (though I can't find the discussion -- maybe it was in person or over email). Unsafe unions are different from My take is that this feature would complicate mypy significantly. Normal union types are already a very complicated feature, and they still aren't even fully implemented. It would be a likely cause of some user confusion, since we'd have two pretty similar but different concepts -- union types and these any/unsafe unions. If we decide to add intersection types at some point, there would be even more potential for confusion, since they are also similar in some ways to any unions. All in all, I don't think that there is enough evidence of the usefulness of the feature to support implementing it. |
AnyUnion
typeAnyUnion
type?)
Solving the original issue is a priority because this may come up often when using ignored imports. I'm not convinced that adding |
Concerning the original example: from typing import Optional
def f():
# untyped function; often from silent import
return 0 # always returns an int
def g(x: Optional[int]) -> int
if x is None:
x = f()
return x what is wrong with x: Union[int, Any]
reveal_type(x) # Revealed type is 'Union[builtins.int, Any]'
x.whatever # Error: Item "int" of "Union[int, Any]" has no attribute "whatever" Also, |
@ilevkivskyi: That doesn't work, as variables don't change type in general. In particular, we want to maintain this behavior: x = 0 # type: int
x = any()
reveal_type(x) # int |
But there is the type binder that can already do this: x: Optional[int]
if x is None:
x = int()
reveal_type(x) # Revealed type is 'builtins.int' Why can't it be special-cased to re-bind to |
Unless there's a really good reason, Unions should work as similarly as possible to non-Unions. I don't think this is a strong enough reason. The binder only binds to subtypes of the declared type. If we change that, the declared type starts to become pretty meaningless. |
Nevertheless the bar for introducing a new concept like AnyUnion is way
higher.
|
Always rebinding a variable to def f(x: Iterable[int]) -> None:
x = make_sparse_vector(x)
x[0] = 1 # not valid for Iterable but okay for vector
def make_sparse_vector(iterable):
... # returns a vector object This currently generates a false positive error. I'd argue that it would be more correct if the example did not generate errors. The type of def g(x: Optional[int]) -> int
if x is None:
x = f()
# type of x is Any here
# type of x is Union[int, Any] here
return x # okay Yes, this change would generate some additional false negatives. It would also fix false positives and be consistent with the philosophy of gradual typing. For gradual typing to work seamlessly, changing a non- |
This seems very attractive. I have one concern though. Suppose I have a file-ish object whose line = b'' # say it's previously used
line = f.readline()
a = line.decode('utf8') # will infer str Or will |
On a second thought, I also think it is not such a terrible idea to rebind to |
Yes. You'd have to use something like Once we can easily calculate the typing precision of a program, it would be easy enough to run an experiment to determine the cost in type checking precision this change would cause for any particular real-world codebase. |
Hm, I really don't like the cast. I've recommended using assignment to a variable combined with a type annotation to avoid casts many times. My reasoning is that if in the future the type of |
Agreed that we shouldn't recommend the cast. Using a temporary variable would be my favorite approach if it's impractical to add an annotation for line = b'' # say it's previously used
next_line = f.readline() # type: bytes
line = next_line
a = line.decode('utf8') # will infer str |
OK, so maybe that would then run up to #2008...? I guess we should double-check that the solution chosen there doesn't invalidate this use case. |
Proposal 3 from #2008 would prevent this solution (or would get very special-casey). I'm not a fan of always rebinding a variable to |
I don't expect this to be the case, as in the steady state codebases are expected to have a high annotation coverage, and for partially annotated code this would only affect a relatively small fraction of assignments. This would have some temporary effect during the migration from originally unannotated code, and even then there would be a trade-off: this would simplify migration by generating fewer false positives at the cost of less precise type checking. Anyway, we can run experiments to estimate the impact, we don't just have to guess. And yes, this would conflict with proposal 3 in #2008 -- it would need tweaks. On the other hand, the def f(x: Iterable[int]) -> None:
x = make_sparse_vector(x)
# type would still be Iterable[int]
x[0] = 1 # ouch
def make_sparse_vector(iterable):
... # returns a vector object We also have the option of using a quick hack that would work around the original issue: special case assignment of from typing import Optional
def f():...
def g(x: Optional[int]) -> int
if x is None:
x = f()
reveal_type(x) # Union[int, Any]
reveal_type(x) # Union[int, Any]
return x # Ok Implementing |
The option number 3 already mentions
as a downside. Maybe I am missing something, but this could be a minor tweak. Something like: types are always bound on initial assignments unless r.h.s. is |
It looks like #3724 is another example where people want to rebind to |
Yeah, it looks like this is a somewhat frequent issue. False positives like this are quite inconvenient for users. The simple workaround modified so that it also fixes #3724 sounds like the best proposal we have for now (in terms of benefit vs cost). |
We should check how many more Anys the simple proposal would introduce into real world codebases before merging it. |
It's not that simple. Take this circumstance, for example:
We don't want to bind |
I think bind |
#3431 is also similar. |
This seems to happen frequently enough that I'm bumping priority to "high". Also removing the "needs discussion" label -- let's special case |
I'm not 100% sure what the proposal is. From "discussed above" link it seems that we should make this work? def f():...
def g(x: Optional[int]) -> int:
if x is None:
x = f()
reveal_type(x) # Union[int, Any] # <---- this is new (currently Optional[int])
reveal_type(x) # Union[int, Any] # Currently also Optional[int]
return x # Ok # Currently an error (Incompatible return value type ...) But how? And should the fix take into account that f() is unannotated, or should it work for anything of type
The action is then apparently to set the target's type to Have I got that? |
I think not much have to be done, the binder would do the things, but it is explicitly prohibited to do many things with # Assigning an Any value doesn't affect the type to avoid false negatives I think we can just add an exception there, so that when |
As a special case, when assigning Any to a variable with a declared Optional type that has been narrowed to None, replace all the Nones in the declared Union type with Any. The result of this is: ``` def f():... def g(x: Optional[int]) -> int: if x is None: x = f() reveal_type(x) # Union[int, Any] # <---- this is new (was Optional[int]) reveal_type(x) # Union[int, Any] # was also Optional[int] return x # Ok # was an error ``` Fixes #3526.
As a special case, when assigning Any to a variable with a declared Optional type that has been narrowed to None, replace all the Nones in the declared Union type with Any. The result of this is: ``` def f():... def g(x: Optional[int]) -> int: if x is None: x = f() reveal_type(x) # Union[int, Any] # <---- this is new (was Optional[int]) reveal_type(x) # Union[int, Any] # was also Optional[int] return x # Ok # was an error ``` Fixes #3526.
Consider this code (run with
--strict-optional
):Mypy doesn't infer that
x
is notNone
ing
. I think this a bug -- mypyshould be permissive here.
If we wanted to fix this, we'd run into a problem: what type should
x
haveafter
x = f()
? We don't want it to beAny
, because we don't want thatbehavior in the general case. It shouldn't be
int
--f()
could be intendedto unconditionally return
None
instead. To fix this, I think we need tointroduce a new special type, which I will call
AnyUnion
.AnyUnion[A, B, C]
would represent a type which is allowed to be used as any ofA
,B
, orC
(similarly to howAny
is allowed to be used as any type). In this case,we could have
x
's type bound toAnyUnion[int, None]
, which would fix thiserror.
Other helpful uses of
AnyUnion
:pow
will no longer need to returnAny
in builtins. Instead, it couldreturn the much less broad
AnyUnion[int, float]
.AnyUnion[X, None]
would provide a per-variable opt-out of strict Nonechecking. This could be helpful for classes with delayed initialization where
a lot of effort would be needed to refactor them into a state that would work
properly with strict None checking.
Cons:
makes things slightly harder for users to understand.
currently works), but
AnyUnion
doesn't help much with that.The text was updated successfully, but these errors were encountered: