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

Clarify aliases vs variables #3494

Closed
ilevkivskyi opened this issue Jun 3, 2017 · 6 comments · Fixed by #8200
Closed

Clarify aliases vs variables #3494

ilevkivskyi opened this issue Jun 3, 2017 · 6 comments · Fixed by #8200

Comments

@ilevkivskyi
Copy link
Member

We have both type aliases and variables with types like Type[...], also there are module aliases and variables of type ModuleType. I think we should document the difference in semantics and provide examples. (Also I think there are few minor things that should be fixed in mypy.) Here how I see things:

  1. Variables with type Type[...] should be created by assignments with an explicit type annotations:
    class A: ...
    tp: Type[A] = A
  2. Aliases are created by assignments without an explicit type:
    class A: ...
    Alias = A
  3. The difference is that aliases are completely known statically and can be used in type context (annotations):
    class A: ...
    class B: ...
    
    if random() > 0.5:
        Alias = A
    else:
        Alias = B  # I think this should be prohibited (there are two TODOs in semanal.py)
    
    tp: Type[object]
    if random() > 0.5:
        tp = A
    else:
        tp = B  # This is OK
    
    def fun1(x: Alias) -> None: ... # This is OK
    def fun2(x: tp) -> None: ... # Error
  4. There is similar distinction between module aliases and variables with type types.ModuleType:
    import math, cmath
    from types import ModuleType
    
    mod: ModuleType = math  # variable
    m = math # Alias
    
    mod.sin # Error. ModuleType has no attribute 'sin'
    m.sin # OK
    and also
    if random() > 0.5:
        m = math
    else:
        m = cmath  # Should be prohibited
    
    mod: ModuleType
    if random() > 0.5:
        mod = math
    else:
        mod = cmath # OK, note explicit annotation above
  5. For modules, we could add better support later (a particular module is a literal type as in Simple dependent types #3062 and binder can bind mod to Union[Literal[math], Literal[cmath]] after the above code).

@JukkaL Do we agree on semantics here? If yes, then I will make some minor changes in mypy and add this to documentation (maybe we also need to clarify this in PEP 484).

@carljm
Copy link
Member

carljm commented Jun 3, 2017

The current version of #3435 allows

if random() > 0.5:
    m = math
else:
    m = cmath

and infers m as ModuleType var (not module alias), even without the explicit prior annotation. I did it this way because of the concern that we could break currently-working code otherwise.

The behavior you propose here would actually be much simpler to implement, as it wouldn't require backtracking. It only requires that we never propagate a module alias to an already-typed variable, which is already implemented.

@ilevkivskyi
Copy link
Member Author

@carljm

The behavior you propose here would actually be much simpler to implement

Probably you are right. Even though this may flag some existing code, this would be actually right. Plus this can be easily fixed by adding explicit annotation - EIBTI.

@JukkaL
Copy link
Collaborator

JukkaL commented Jun 5, 2017

@ilevkivskyi I mostly agree, though I'm not sure about using literal types for modules -- they feel different to me. Anyway, better types for modules should probably be discussed separately from the rest of the points. Feel free to open an issue for this -- I couldn't find an existing issue.

@carljm I agree with Ivan -- requiring an explicit annotation would be the right thing in case of a conditional module alias. This would be consistent with how mypy behaves in other contexts.

@ilevkivskyi
Copy link
Member Author

@JukkaL

I'm not sure about using literal types for modules -- they feel different to me. Anyway, better types for modules should probably be discussed separately from the rest of the points. Feel free to open an issue for this -- I couldn't find an existing issue.

Yes, I will open a separate issue. This was just a vague idea.

@ilevkivskyi
Copy link
Member Author

ilevkivskyi commented Jun 6, 2017

OK, opened a separate issue #3500 for point 5 of the list in original post.

EDIT: Corrected the issue number.

@carljm
Copy link
Member

carljm commented Jun 6, 2017

The "better types for modules" issue is #3500.

@ilevkivskyi ilevkivskyi self-assigned this Jun 7, 2017
JukkaL pushed a commit that referenced this issue Jul 7, 2017
Fixes #2887. Fixes #3191.

In addition this prohibits reassigning aliases. Previously something like 
this was allowed:

```
if random():
    Alias = Sequence[int]
else:
    Alias = Sequence[float]

def fun(arg: Alias) -> None:
    ...
```

Now this will generate an error: 

    Cannot assign multiple types to name "Alias" without an explicit "Type[...]" 
    annotation. 

See #3494 for background.

Finally, this simplifies the logic in semanal.py, so that most processing of type aliases 
happens in one function.
@ilevkivskyi ilevkivskyi removed their assignment May 14, 2018
facebook-github-bot pushed a commit to facebook/pyre-check that referenced this issue Jan 30, 2019
Summary:
EDIT: first version was based on wrong assumptions - aliases and globals are separate concepts, differentiated by whether there is an explicit annotation
python/mypy#3494

-----

Some examples where we rely on globals typed as `Any` (they might have some other value on the RHS of the assign) to be valid types:
diffusion/IGSRVHG/browse/master/distillery/gatekeeper/restraints/checks/request/app_os.py$36,45#
https://fburl.com/biggrep/rmd6ufa4

Typeshed mock.pyi uses this for some of its classes:
https://github.com/python/typeshed/blob/0730fe5fcb269df3f1b72e2544671fec1498a6db/stdlib/3/unittest/mock.pyi

Reviewed By: sinancepel

Differential Revision: D13858398

fbshipit-source-id: 290c5d9644c88c52082c58c052ad9a8efe172d64
JukkaL pushed a commit that referenced this issue Jan 30, 2020
Resolves #3494 (Since module types are tracked in #3500)

Following #8187 (comment) and #3494, if I understand correctly and the semantics 
in #3494's example code has been fixed (report error on re-assign Alias = B, the 
remaining work of #3494 is to update the docs, which is the main focus of this PR.

Newly added docs are in common issues and solutions section, with the content mostly adapted from Ivan's example in #3494. And a note point to the docs is also added.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants