Skip to content
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

Inference from unannotated functions #4455

Open
elazarg opened this issue Jan 10, 2018 · 7 comments
Open

Inference from unannotated functions #4455

elazarg opened this issue Jan 10, 2018 · 7 comments
Labels
bug mypy got something wrong needs discussion

Comments

@elazarg
Copy link
Contributor

elazarg commented Jan 10, 2018

Mypy uses the body of unannotated functions for inference and to understand what attributes an object has. For example:

d = {}
e = {}

def f():
    d['x'] = 1

d['y'] = 2
e['y'] = 2

reveal_type(d)  # dict[Any, Any]
reveal_type(e)  # dict[str, int]

I couldn't find any mention of it on the docs. I think this behaviour needs to be documented. Perhaps as a separate section, to make it searchable.

Related: #4434 #4409

@ilevkivskyi
Copy link
Member

ilevkivskyi commented Jan 10, 2018

I would actually say this is a bug. We shouldn't use Any types (even explicit probably) for partial types, and require an annotation instead (if there are no other assignments in scope). This can lead to false negatives:

d = {}  # We should require an annotation here, if the second assignment below is absent...

def g():
    d['x'] = 1

d[1] = 'x'  # ...instead of inferring 'Dict[Any, Any]' that may be unsafe.

@ilevkivskyi ilevkivskyi added bug mypy got something wrong needs discussion labels Jan 10, 2018
@ilevkivskyi ilevkivskyi changed the title Document inference from unannotated functions Inference from unannotated functions Jan 10, 2018
@elazarg
Copy link
Contributor Author

elazarg commented Jan 10, 2018

IMO it is unclear that the error is at the last line. I would expect that line to define the type, and the error to be inside the function, since it is unannotated and therefore assumed to be too clever for the type checker.

I am confused though. I agree that we shouldn't use (implicit) Any for partial types, but from the discussion on #4434 I understood that this is by design.

This issue was meant to be more general: I am trying to understand how things should behave. What are the rules.

@ilevkivskyi
Copy link
Member

IMO it is unclear that the error is at the last line.

I was probably not very clear in my example.

At least one of the two assignments is probably erroneous. But it would be safer to use the second for inference. The only downside of this is that after annotating the function, an error will appear in previously valid code. But this is better than the current (clearly unsafe) behavior. In case if there is no second assignment we should give a normal Need type annotation for variable error.

I am confused though. I agree that we shouldn't use (implicit) Any for partial types, but from the discussion on #4434 I understood that this is by design.

I don't think current behavior is really intentional, @JukkaL may correct me.

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 11, 2018

The current behavior seems accidental or at least not very well thought out. I think that it would be better to require all parts of a partial type definition to be located within a single scope. So this shouldn't be valid:

d = {}  # We should require an annotation here

def f():
    # This is a different scope from the initial assignment so can't affect the partial type
    d['x'] = 1

In the original example, we should probably use the second assignment for inference (d['y'] = 2). There probably shouldn't be an error, since the function is unannotated and we can just give d type Any within the function, independent of whether it has a partial type or not. If the function is annotated, we can defer the function and re-check it after we've finished with the module top level. This would conform to how fine-grained incremental works, I think.

@elazarg
Copy link
Contributor Author

elazarg commented Jan 11, 2018

Jukka, I agree. So - what does mypy do inside unannotated functions?

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 11, 2018

Here are some things (probably incomplete):

  • Mypy infers the existence of attributes through self assignments.
  • Mypy follows imports within unannotated functions.
  • Mypy does some reachability analysis (sys.version_info checks, for example), but this seems inconsistent.
  • Does some analysis of nested, annotated functions (again this seems inconsistent).
  • As discussed above, it does some partial type related analysis, but this should probably be removed.

Much of this isn't very principled, to be honest.

@elazarg
Copy link
Contributor Author

elazarg commented Jan 11, 2018

Seems like the not-inconsistent part is straight-forward semantic analysis.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug mypy got something wrong needs discussion
Projects
None yet
Development

No branches or pull requests

3 participants