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

"already defined" errors when adding type annotations in multiple code paths #1191

Closed
timabbott opened this issue Jan 29, 2016 · 17 comments
Closed

Comments

@timabbott
Copy link

While trying to debug something, I ended up adding type annotations for the definitions of an object in both code paths, and got a /home/tabbott/foo.py:10: error: Name 'child_pid' already defined with this (simplified) code:

if True:
    child_pid = os.fork() # type: int
    # Do stuff
else:
    child_pid = None # type: int

Since the types are the same, it seems like this shouldn't be an error.

This may be a variant of #1153.

@gvanrossum
Copy link
Member

Hm, I think mypy actually has an intentional rule that you shouldn't annotate a variable twice, and I like that rule. So I am tempted to reject this one. The right fix would be to make the first annotation Optional[int].

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 29, 2016

Yeah, this is by design.

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 4, 2016

Closing this now. Feel free to reopen if you don't agree :-)

@JukkaL JukkaL closed this as completed Feb 4, 2016
@ruuda
Copy link

ruuda commented Jul 11, 2018

Somewhat related, this error also triggers even when del is used:

if True:  # For example, should not be const in practice.
    foo: int = 0
    print(foo)
    del foo
else:
    foo: int = 0
    print(foo)

In the following example it even reports an error: "Trying to read deleted variable 'foo'" on the last line.

if True:  # For example, should not be const in practice.
    foo: int = 0
    print(foo)
    del foo

foo: int = 0
print(foo)

I am using Mypy 0.610.

@gvanrossum
Copy link
Member

There should be only one declaration of a given variable per scope. And a function is one scope.

@impredicative
Copy link

There should be only one declaration of a given variable per scope.

This recommendations seems wholly unreasonable. Python itself has no such restriction. I feel lousy having to change my variable names just to make mypy happy.

@gvanrossum
Copy link
Member

We are actively working on this restriction.

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 15, 2019

The initial implementation will likely only cover redefinition within a single block at the same nesting level, but it's possible that we'll extend it to handle other cases in the future.

@komuw
Copy link

komuw commented Mar 9, 2019

The initial implementation will likely only cover redefinition within a single block

@JukkaL should this github issue be re-opened?

I just ran into this is in my code.

import typing, logging

def convertLog(log_level: typing.Union[str, int]) -> int:
    """
    usage:
       level = convertLog(log_level="DEBUG") 
       assert level == 10
    """
    if isinstance(log_level, str):
        log_level: int = logging._nameToLevel[log_level.upper()]
    return log_level

mypy complains about log_level already defined.
However, do note that a log_level enters the function either as an int or a str but can only come out as an int

@ilevkivskyi
Copy link
Member

@komuw You can simply remove that : int annotation, it is just not needed there.

@bela127
Copy link

bela127 commented Feb 23, 2020

Same Problem in the flowing to implementations
Note that implementation 2 is the way of annotating for loops per pep

so it definitely should be supported to have for loops with different types for the running variable

class Functionality():
   
    def __init__(self: Functionality, func: Callable[Info,Any]):
        super().__init__()
        self.func: Callable[Info,Any] = func
        self.pre_call: List[PreExpert] = []
        self.post_call: List[PostExpert] = []
        
    def __call__(self: Functionality, info):
      
        for expert in self.pre_call:
            if expert.can_work_with(info):
                expert.execute(info)
        
        ret = self.func(info)

        for expert in self.post_call:                    ##Problem
            if expert.can_work_with(info):
                expert.execute(info)
                
        return ret
class Functionality():
   
    def __init__(self: Functionality, func: Callable[Info,Any]):
        super().__init__()
        self.func: Callable[Info,Any] = func
        self.pre_call: List[PreExpert] = []
        self.post_call: List[PostExpert] = []
        
    def __call__(self: Functionality, info):
        expert: PreExpert
        for expert in self.pre_call:
            if expert.can_work_with(info):
                expert.execute(info)
        
        ret = self.func(info)
        
        expert: PostExpert  ##Problem
        for expert in self.post_call:
            if expert.can_work_with(info):
                expert.execute(info)
                
        return ret

@JukkaL
Copy link
Collaborator

JukkaL commented Feb 28, 2020

You can try --allow-redefinition -- it will let you redefine some variables (though not always).

@DevilXD
Copy link

DevilXD commented Mar 3, 2020

I'm having an issue where this redefinition check ignores pathways that will never happen at runtime, for example where the code takes a path after an if check, that ends with a return. Simplified example:

key: Union[int, str]

if isinstance(key, int):
    ...
    result: Tuple[int, str] = ...
    ...
    return result
...  # here the type of 'key' is 'str' only
result: Tuple[int, str] = ...  # here is the redefinition error, pointing at the line inside the if check above as the original definition
...
return result

@cipollone
Copy link

cipollone commented Feb 17, 2021

I'm fine with the rule "only one annotation per scope". To me, the problem is just that the error is not informative enough. Both the error text and the example in the documentation suggest that it may be a coding issue. Instead, I just had to remove the second annotation!

if not resuming:
  # Initialize
  obj = MyClass()
else:
  # Load back
  obj: MyClass = loader.deserialize()

Without this github issue I couldn't figure out what was wrong.

@JulesGM
Copy link

JulesGM commented Jun 29, 2022

any progress on removing the prohibition of declaring the same variable multiple times? this is annoying, should be left to the user

@parhamfh
Copy link

Stumbled upon this as well, perhaps if this is helpful to some but not to others it could be turned into a warning instead. This feels like checking code style.

@bafulton
Copy link

bafulton commented Feb 6, 2023

I just stumbled onto this now. I agree with @JulesGM and @parhamfh --this feels too prescriptive to me. I'd be in favor of making it a warning.

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

No branches or pull requests