Cirq
Cirq copied to clipboard
Consider `--allow-redefinition` for mypy
See https://github.com/python/mypy/issues/1174 for full context, but there are common idioms where you may want to re-use a variable name
items: Set[Item] = set()
items.add(...)
items = frozenset(items)
Mypy currently complains. They added a flag: https://github.com/python/mypy/pull/6197 which may or may not become the default at some point. I say we add this to our mypy configuration.
ref https://github.com/quantumlib/Cirq/issues/3767
I have mixed feelings about this. Sometimes it's nice to redefine variables (I often encounter this when a function accepts an arg with a union type but we then convert it to a single type and want the rest of the code to use this "narrowed" type). But other times a redefinition is inadvertent and mypy flagging it can actually detect bugs (sometimes bugs like this only surface when refactoring). It looks like --allow-redefinition
only applies in certain limited situations, which is good, but I'd still be a bit leery about adding it.
I'm going through the numpy stuff and there are a non-zero number of places where redefinitions are used, especially when constructing arrays:
arr: List[float]
for thing in iterator:
arr.append(complicated_function(thing))
arr: np.ndarray = np.asarray(arr)
Discussion in Cirq sync:
- particularly nice for e.g. redefining
Optional
values to non-optional type - not allowing redefinition enforces cleaner code, but mostly helps with redefinition in multiple scopes
From mypy: --allow_redefinition
only allows redefinition within the same block and nesting depth.