You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Thanks, it's a bit better. There's still this case that doesn't seem to
work right:
import sys
if sys.platform == 'darwin':
import os
else:
os = None
I get an error on each line mentioning os:
/Users/guido/mypy_tests/mypy_imp.py:3: error: Need type annotation for
variable
/Users/guido/mypy_tests/mypy_imp.py:5: error: Name 'os' already defined
On Fri, Nov 27, 2015 at 1:58 PM, Jukka Lehtosalo [email protected]
wrote:
Now some more conditional definitions are accepted by mypy. These are all
okay:
try:
from m import CONST
except ImportError:
CONST = 'const'
try:
from m import f
except ImportError:
def f(): ...
def f(): ...
try:
from m import f
except ImportError: pass
There's still a lot more work to do before mypy can deal with all
interesting cases, but this should cover a good fraction of cases that used
to fail.
—
Reply to this email directly or view it on GitHub #649 (comment).
Since it's the exact opposite of one that does work, how hard would it be
to make it work?
(In general I am beginning to become more interested in more symmetrical
unification of branches when the type info gathered from one branch is of a
clearly subordinate nature -- similar to x = [] if cond else [1, 2, 3],
which IMO should infer a type of List[int] for x regardless of context.)
I don't expect that supporting the f = None / def f(): ... case would be hard once we support f = g / def f(): ....
[Here's some background for why these are hard. Mypy treats function definitions (and modules and classes) as special and different from variables. This is important for modules as it allows expressions like mod.ClassName to be bound early during semantic analysis, but less so for functions. We could unify functions and variables, but it would be pretty big change and there are some benefits to how things work currently.]
Maybe it could give up on that special treatment whenever it sees one of
these idioms, and treat the variable more as something of a given (maybe
optional) type?
Okay, getting back to the original reported issue -- the fixes above don't actually solve it, as it also has the feature of both using a builtin in a module and redefining it. However, now there is fairly simple (though perhaps quite non-obvious) workaround:
As we've pretty much implemented all the low hanging fruit improvements, I'm downgrading priority. Let me know if there are still common issues and I'll bump priority again.
We could also close this issue and create separate issues for the remaining unsupported cases.
Activity
refi64 commentedon Apr 20, 2015
This is a hack...but, whenever I encounter stuff like this, I do:
JukkaL commentedon Apr 29, 2015
This is a known, long-standing issue. Conditional definitions are often rejected because mypy is too picky.
A potential fix would be to treat
defmore like an assignment, rather than a special 'function definition'.JukkaL commentedon Nov 25, 2015
Another, slightly different example:
JukkaL commentedon Nov 25, 2015
Merging with #212, which is similar.
Merge branch 'conditional-definition'
JukkaL commentedon Nov 27, 2015
Now some more conditional definitions are accepted by mypy. These are all okay:
There's still a lot more work to do before mypy can deal with all interesting cases, but this should cover a good fraction of cases that used to fail.
gvanrossum commentedon Nov 28, 2015
Thanks, it's a bit better. There's still this case that doesn't seem to
work right:
I get an error on each line mentioning os:
On Fri, Nov 27, 2015 at 1:58 PM, Jukka Lehtosalo [email protected]
wrote:
--Guido van Rossum (python.org/~guido)
JukkaL commentedon Nov 28, 2015
That should be pretty easy to support as well.
gvanrossum commentedon Nov 28, 2015
gvanrossum commentedon Dec 11, 2015
OK, another case found in the wild:
JukkaL commentedon Dec 11, 2015
I also have a long, unprocessed list of related issues from the Python 2.7 std library that I should analyze one of these days.
Merge branch 'import-and-assign-none'
JukkaL commentedon Dec 31, 2015
These now work:
These still don't work:
gvanrossum commentedon Jan 5, 2016
Here's another one that still doesn't work:
Since it's the exact opposite of one that does work, how hard would it be
to make it work?
(In general I am beginning to become more interested in more symmetrical
unification of branches when the type info gathered from one branch is of a
clearly subordinate nature -- similar to
x = [] if cond else [1, 2, 3],which IMO should infer a type of
List[int]for x regardless of context.)JukkaL commentedon Jan 6, 2016
I don't expect that supporting the
f = None/def f(): ...case would be hard once we supportf = g/def f(): ....[Here's some background for why these are hard. Mypy treats function definitions (and modules and classes) as special and different from variables. This is important for modules as it allows expressions like
mod.ClassNameto be bound early during semantic analysis, but less so for functions. We could unify functions and variables, but it would be pretty big change and there are some benefits to how things work currently.]gvanrossum commentedon Jan 6, 2016
JukkaL commentedon Jan 8, 2016
Treating a function definition like an assignment if it's already defined as a variable would work. Consider code like this:
We would treat it like this code:
Merge branch 'more-redefine-function'
Merge branch 'assign-none-to-module'
JukkaL commentedon Jan 8, 2016
These now work:
The
m = None/import mcase is harder to support. I gave it a try but we should have a new type for modules for that.JukkaL commentedon Jan 8, 2016
Okay, getting back to the original reported issue -- the fixes above don't actually solve it, as it also has the feature of both using a builtin in a module and redefining it. However, now there is fairly simple (though perhaps quite non-obvious) workaround:
As we've pretty much implemented all the low hanging fruit improvements, I'm downgrading priority. Let me know if there are still common issues and I'll bump priority again.
We could also close this issue and create separate issues for the remaining unsupported cases.
Merge branch 'nested-function-none-init'
JukkaL commentedon Jan 8, 2016
Closing this now, even though this is not fully fixed. The above issues cover some remaining missing features.