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

Can't index named tuple by defined constant #3078

Closed
gvanrossum opened this issue Mar 29, 2017 · 17 comments
Closed

Can't index named tuple by defined constant #3078

gvanrossum opened this issue Mar 29, 2017 · 17 comments
Labels

Comments

@gvanrossum
Copy link
Member

Example:

from typing import *
A = NamedTuple('A', [('x', int), ('y', str)])
X = 0
Y = 1
a = A()
reveal_type(a[X])  # expected int, but is "Tuple index must be an integer literal"
reveal_type(a[Y])  # expected str, but same error

There's a real-world use case for this: the stats stdlib module defines constants like ST_MODE that are supposed to be used as indices into os.stat_result (which was turned into a NamedTuple in python/typeshed#1103).

@JelleZijlstra
Copy link
Member

A simple fix would be to allow any indexing on NamedTuples and return Any. That would have some false negatives, but I feel like those are unlikely in real code.

@pkch
Copy link
Contributor

pkch commented Mar 29, 2017

The type information is available in TypeChecker.type_map when a[X] is being checked, and it would have been easy to verify that X is of type builtins.int.

But of course TypeChecker.visit_expression_stmt delegates to ExpressionChecker.accept the validation of expressions. And once we're inside ExpressionChecker.accept, the type_map is no longer available. What's a good way to provide the type_map information to ExpressionChecker without creating spaghetti code?

@elazarg
Copy link
Contributor

elazarg commented Mar 29, 2017

(Obviously related to singleton types; #3062)

@gvanrossum
Copy link
Member Author

The type of X is not the problem. We need its value (in this case, where it's a constant) so that we can tell that the type of a[X] is int but the type of a[Y] is str -- just as it does for literals. Here's an updated example:

from typing import *
A = NamedTuple('A', [('x', int), ('y', str)])
X = 0
Y = 1
a = A()
reveal_type(a[0])  # int
reveal_type(a[1])  # str
reveal_type(a[X])  # expected int, but get an error
reveal_type(a[Y])  # expected int, but get an error

@elazarg
Copy link
Contributor

elazarg commented Mar 29, 2017

But if the type of X was the singleton type Literal[0] there was no problem.

@pkch
Copy link
Contributor

pkch commented Mar 29, 2017

Ah right of course, it's the value of X that we need, my bad.

If we end up using Literal[0] types (which I think is a good solution), then my question is still valid: how do we pass type information about type of X from TypeChecker to ExpressionChecker.accept?

If we don't have Literal[0], then perhaps TypeChecker can remember the values of NameExpr (rather than just their types) when they are first assigned; again, somehow this information must flow to ExpressionChecker.

The initial assignment should be enough, since we obviously won't support this:

A = NamedTuple('A', [('x', int), ('y', str)])
X = 0
X = X + 1
a = A()
reveal_type(a[X])  # perhaps Any, with a warning that type cannot be inferred due to second assignment

@gvanrossum
Copy link
Member Author

It's an interesting change to mypy's architecture.

I also worry about how mypy would tell that ST_MODE is meant to be a constant -- there's no Python syntax to distinguish between a constant and e.g. a global counter, and the naming convention (UPPER_CASE for constants) is pretty weak and feels unreliable.

@pkch
Copy link
Contributor

pkch commented Mar 29, 2017

Can mypy assume it's a constant if the variable (properly qualified of course) is assigned (or augmented assigned) to only once across the entire codebase?

Edit: assigned to only once, and never augmented assigned I meant.

@elazarg
Copy link
Contributor

elazarg commented Mar 29, 2017

It feels like we are trying to reinvent abstract interpretation,

@pkch
Copy link
Contributor

pkch commented Mar 30, 2017

@gvanrossum

That's easy for locals, but what about attributes or class vars? Any time you call anything it may change one of those:

class C:
    def f(self) -> None:
        self.foo = True
        self.bar()
        # Here we don't know if self.foo is still True
    def bar(self): ...  # A subclass may override

Well, both local and global variables are ok.

I think mypy would know whether a class attribute is ever modified in the codebase (because it keeps track of object type, and modifying a class attribute requires the base of the member access to be of that type).

That instance attributes. Maybe just not consider them as constants, ever? I think there are fewer use cases where someone needs to access named tuples fields using instance attributes.

@JukkaL
Copy link
Collaborator

JukkaL commented Mar 30, 2017

@pkch Mypy often doesn't see the entire codebase (due to stubs, skipped modules, etc.) so it can't decide whether something is assigned to only once reliably. More importantly, mypy does no whole-program reasoning because it makes incremental checking really hard and can result in undesirable spooky-action-at-a-distance behavior.

@pkch
Copy link
Contributor

pkch commented Mar 30, 2017

In that case, maybe we should allow some syntax to declare a constant?

@JukkaL
Copy link
Collaborator

JukkaL commented Mar 30, 2017

That would probably the best way to do this. This actually comes up pretty frequently, so it might be a feature worth considering. A new syntax may require a PEP 484 change in order for it to be usable in stubs, which makes adoption harder. Here are some ideas for the syntax:

  • X = 2 # type: Const or X = 2 # type: Const[int] (these would be equivalent in this case)
    • Alternatively, Final[x] as in Java
  • X = 2 # mypy: const or X = 2 # type: int # mypy: const (this wouldn't require a PEP 484 change, but this is more verbose and less consistent with the rest of the syntax)
  • X = Const(2) (this Const could live in mypy_extensions so it wouldn't require a PEP 484 change)

Here's another alternative that I don't like:

__const__ = ['X']   # Like __all__
X = 1

I'm not yet convinced that this is a high-priority feature, but it might be worthwhile to have eventually.

@gvanrossum
Copy link
Member Author

I kind of like Final. My preferred syntax would then be

x: Final = 0
y: Final[int] = 0

With fallbacks using # type: Final and # type: Final[int].

@ilevkivskyi Would it be absolutely necessary to make changes to typing.py for this? Couldn't it live in mypy_extensions.py?

@ilevkivskyi
Copy link
Member

Would it be absolutely necessary to make changes to typing.py for this? Couldn't it live in mypy_extensions.py?

I don't see any strong reasons to put it in typing. Of course implementing it in mypy_extensions will be a bit more difficult, but it is a very minor difference.

There is another point that comes to my mind: If we will have several things only in mypy_extensions , several things only in typing, and several things in both, then people could simply forget where to look for a particular type. Maybe we could overcome this problem by having documentation for mypy_extensions (IIRC it is completely undocumented now).

@gvanrossum
Copy link
Member Author

I think of mypy_extensions as a way to have quicker turn-around for things that ought to go in typing, in addition to a place where experiments or extensions that will always be mypy-specific go. In my ideal world everything in mypy_extensions would eventually go into typing, but having it in mypy_extensions would provide a backwards compatible solution for people who need to be portable to old 3.5 or 3.6 versions.

So the issue you bring up will exist, but it's acknowledged that it's a temporary solution.

@ilevkivskyi
Copy link
Member

This is now supported with literal types.

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

No branches or pull requests

6 participants