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

Dynamic base classes are not handled #2477

Closed
snakescott opened this issue Nov 22, 2016 · 28 comments
Closed

Dynamic base classes are not handled #2477

snakescott opened this issue Nov 22, 2016 · 28 comments

Comments

@snakescott
Copy link

snakescott commented Nov 22, 2016

repro:

from sqlalchemy import Column, String
Base = declarative_base()

class Foo(Base):
  __tablename__ = 'foo'
  key = Column(String, primary_key=True)
  • install mypy 0.4.6
  • run mypy --py2 -v -v -v --fast-parser -s --custom-typeshed-dir $BASEDIR/typeshed $BASEDIR/failure.py

Output is something like

failure.py:33: error: Invalid type "failure.Base"
failure.py:33: error: Invalid base class

when it should be clean

@gvanrossum
Copy link
Member

gvanrossum commented Nov 24, 2016

Simpler repro (without the typeshed/sqlalchemy stuff):

def func(): pass
Base = func()
class C(Base): pass  # errors on this line

Errors (mypy -s x.py):

x.py:3: error: Invalid type "x.Base"
x.py:3: error: Invalid base class

(Update: used def func... instead if import.)

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 24, 2016

The reason this doesn't work is that the type of Base is not yet inferred during semantic analysis, and that's when we process base classes. You can add an annotation as a workaround:

from typing import Any

def func(): pass
Base = func()  # type: Any
class C(Base): pass  # now works

@gvanrossum
Copy link
Member

Should we close this then or update the docs somewhere? Maybe the error message could also be better.

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 24, 2016

I think that we should at least see if we can make the error message better.

@snakescott
Copy link
Author

improved error message would be helpful. I did discover # type: Any as a workaround but it is good to be aware this is a known issue.

Is there a github issue I should be tracking for the semantic analysis improvement, or just watch the release notes?

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 16, 2017

Let's use this issue to track both the error message and the underlying semantic analysis issue for now.

@Deimos
Copy link
Contributor

Deimos commented Jan 19, 2017

I seem to have hit a very similar bug that's a bit more convoluted (involving Optional and importing), but probably has a related base cause. It happened to me originally with sqlalchemy's declarative_base() as well, but this was my attempt to find a simple reproduction (Python 3.6):

file1.py:

from typing import Any

Base: Any

file2.py:

from typing import Optional
from .file1 import Base

class SubClass(Base):
    pass

def optional_return_func() -> Optional[SubClass]:
    return SubClass()

def should_fail() -> int:
    test_var = optional_return_func()
    reveal_type(test_var)

    if not test_var:
        return 1

    return 'not an int'

When I run mypy on this, the result is error: Revealed type is 'builtins.None'

The expected result should be Revealed type is 'Union[file2.SubClass, builtins.None]' along with an Incompatible return value type error, but since mypy is certain that test_var is None, it considers the final line that returns a string unreachable.

Hope that makes sense, let me know if any more information would be helpful.

@Deimos
Copy link
Contributor

Deimos commented Jan 30, 2017

@JukkaL sorry for the ping, but I just wanted to check and see if you think the issue I described above is similar enough that leaving it as part of this one is good, or if I should open a new, separate issue for it.

@JukkaL
Copy link
Collaborator

JukkaL commented Jan 31, 2017

@Deimos The builtins.None issue seems to be fixed on github master -- can you verify that it works for you? It's unrelated to the original issue, but no need to create an issue if it already works.

@Deimos
Copy link
Contributor

Deimos commented Jan 31, 2017

Oh, yes, it does seem to be fixed using current github master. That's great, thanks (and sorry for bothering you about something that was already fixed).

@libre-man
Copy link

But if I add a Any annotation the problem is that it thinks the class can be anything, so doing stuff like

import typing as t
base: t.Any = make_base()
class A(base): pass

def make_list() -> t.Sequence[A]:
    return A()

does not yield an error which makes anything using this class (A in this case) not typesafe anymore.

@gvanrossum gvanrossum changed the title MyPy not handling sqlalchemy declarative_base correctly Dynamic base classes are not handled Jul 9, 2017
@gvanrossum
Copy link
Member

But what would you expect? Except for a few trivial edge cases, when the base class is computed dynamically, mypy is unable to understand the structure of your class. That's why it gives an error when you use a dynamic base class -- if you shut it up using Any, literally "Any"thing can happen. :-)

[I changed the issue topic because it has nothing to do with sqlalchemy.]

@libre-man
Copy link

I understand that mypy can't determine the class, however it might be nice to give mypy a typehint of some sort. Capturing all the possibilities of the base are probably still not possible, however it would be possible to check somethings.

I suspected that doing Type[cls] would do this, but this does not work. This also feels somewhat inconsistent as

class A: pass
Base = A
Base2: Type[A] = A
class B(Base): pass
class C(A): pass
class D(Base2): pass

only gives an error for class D but passing A to a function like def e(cls: Type[A]) works as expected.

@gvanrossum
Copy link
Member

If you have a base class that you think would make for a suitable static substitute for the dynamic base, you could try this (I am using object here for the static substitute):

base = object
def make_base(): pass
base = make_base() # type: ignore
class C(base):
  def foo(self): pass
c = C()
c.foo()
c.bar()  # Error, proving static type checking honors the `base = object`.

You might also do something with typing.TYPE_CHECKING:

from typing import TYPE_CHECKING
def make_base(): pass
if TYPE_CHECKING:
  base = object
else:
  base = make_base()
class C(base):
  def foo(self): pass
c = C()
c.foo()
c.bar()  # Error again

@ilevkivskyi
Copy link
Member

@libre-man

This also feels somewhat inconsistent

It is important to make a distinction between type aliases and variables with type Type[...]:

B = A  # makes a type alias
C: Type[A]  # creates a variable

The point is that for aliases we know the value statically since type alias cannot be re-bound, so that it can be used in the type context (like an annotation or a base class). In contrast we don't know the value of a variable statically. Consider this example:

class C1: ...
class C2: ...

A: Type[Union[C1, C2]]

class B(A): ...

it is not clear what would this mean, since one cannot subclass a union.

I like the idea of using TYPE_CHECKING proposed by Guido, since it clearly indicates the purpose.

@w0rp
Copy link

w0rp commented Sep 5, 2017

I was looking for an existing GitHub issue for a problem I was having, and I think this might be the one. I've been trying to define types for Django where the stub files use generic types for Manager and QuerySet, even though the real types currently do not. It works pretty well for me, but I have encountered a problem with trying to define the from_queryset method, which is commonly used for creating Django Manager classes from QuerySet classes, with most of the methods copied across. The code looks roughly like this.

from typing import Any, Generic, TypeVar

ModelType = TypeVar('ModelType')


class Foo:
    pass


class QuerySet(Generic[ModelType]):
    pass


class Manager(Generic[ModelType]):
    @classmethod
    def from_queryset(cls, queryset_class: QuerySet[Any]) -> 'Manager[ModelType]':
        ...


class Bar(Manager.from_queryset(QuerySet)):
    pass

That's enough code to reproduce the problem. mypy will complain with the "invalid base class" error for class Bar.

So far, my workaround is to define Manager and QuerySet classes the old way, by defining methods in two places.

@gvanrossum
Copy link
Member

gvanrossum commented Sep 5, 2017 via email

@w0rp
Copy link

w0rp commented Sep 5, 2017

No sorry, I just thought I'd mention my use case here in case any other Django users notice the same issue I did.

@boompig
Copy link

boompig commented May 30, 2018

Any updates on this issue?

@germaniumhq
Copy link

This also does not work:

def model(base: Any) -> Callable[...,T]:
    class ModelProxy(base):  # fails
        # ....

While this does:

def model(base: Any) -> Callable[...,T]:
    wut: Any = base
    class ModelProxy(wut):  # works
        # ....

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 2, 2018

@germaniumhq Created #5865 to track that particular issue.

@ilevkivskyi
Copy link
Member

Currently mypy supports writing plugins, including using get_dynamic_class_hook(), see the docs. In particular, SQLAlchemy plugin already supports the definitions like in the original example. I am not sure about the Django plugin, if it doesn't support this yet, you can open an issue on their tracker.

I don't think we need to keep this open. One should either use plugins for supported frameworks, or the if TYPE_CHECKING: ... workaround for not yet supported ones.

redshiftzero added a commit to redshiftzero/securedrop-client that referenced this issue Apr 19, 2019
redshiftzero added a commit to freedomofpress/securedrop-client that referenced this issue Apr 26, 2019
* ignore missing imports

https://mypy.readthedocs.io/en/latest/running_mypy.html#ignore-missing-imports

* mypy doesn't handle dynamic base classes

see workaround in python/mypy#2477

* rework language_code handling to resolve TypeError

mypy was flagging this logic due to the fact that current_locale
is Optional[str]

* TimedRotatingFileHandler delay arg should be bool
redshiftzero added a commit to freedomofpress/securedrop-client that referenced this issue Apr 26, 2019
* ignore missing imports

https://mypy.readthedocs.io/en/latest/running_mypy.html#ignore-missing-imports

* mypy doesn't handle dynamic base classes

see workaround in python/mypy#2477

* rework language_code handling to resolve TypeError

mypy was flagging this logic due to the fact that current_locale
is Optional[str]

* TimedRotatingFileHandler delay arg should be bool
redshiftzero added a commit to freedomofpress/securedrop-client that referenced this issue Apr 26, 2019
* ignore missing imports

https://mypy.readthedocs.io/en/latest/running_mypy.html#ignore-missing-imports

* mypy doesn't handle dynamic base classes

see workaround in python/mypy#2477

* rework language_code handling to resolve TypeError

mypy was flagging this logic due to the fact that current_locale
is Optional[str]

* TimedRotatingFileHandler delay arg should be bool
redshiftzero added a commit to freedomofpress/securedrop-client that referenced this issue Apr 27, 2019
* ignore missing imports

https://mypy.readthedocs.io/en/latest/running_mypy.html#ignore-missing-imports

* mypy doesn't handle dynamic base classes

see workaround in python/mypy#2477

* rework language_code handling to resolve TypeError

mypy was flagging this logic due to the fact that current_locale
is Optional[str]

* TimedRotatingFileHandler delay arg should be bool
br3ndonland added a commit to ProjectCheshire/graphql-python-starter that referenced this issue Jun 2, 2019
@revmischa
Copy link

If mypy can't parse it, it is unhelpful to say it's a type error

@w0rp
Copy link

w0rp commented Jun 18, 2019

In case others are interested in the Django type checking issues I mentioned here and they find this thread, I'd like to point to this plugin for mypy: https://github.com/mkurnikov/django-stubs I haven't started using it yet, but it should solve a lot of Django type checking issues.

@ghost
Copy link

ghost commented Jul 11, 2019

Edit: Please disregard, I was able to use TestCase after all.

I can't get this to work:

if TYPE_CHECKING:
    test_mixin_class = TestCase
else:
    test_mixin_class = object


class CanCreateTestMixin(test_mixin_class):

I get

Invalid base class

on the class line with mypy 0.711 and the following configuration:

[mypy]
check_untyped_defs = true
disallow_untyped_defs = true
ignore_missing_imports = true
no_implicit_optional = true
warn_redundant_casts = true
warn_return_any = true
warn_unused_ignores = true

@blueyed
Copy link
Contributor

blueyed commented Nov 7, 2019

@dms-cat
Is this inside of a function?

Note the workaround from Guido above does not work when wrapped in a function in general:

def foo():
    from typing import TYPE_CHECKING
    def make_base(): pass
    if TYPE_CHECKING:
      base = object
    else:
      base = make_base()
    class C(base):  # !! Variable "base" is not valid as a type
      def foo(self): pass
    c = C()
    c.foo()
    c.bar()  # Error again

Simpler:

if True:  # works
    base = object

    class C(base):
        pass


def f():
    base = object

    class C(base):  # Variable "base" is not valid as a type / Invalid base class "base"
        pass

(0.750+dev.a94e649de794ecd169a5bcc42274e5205ffbb1fb)

kdrag0n added a commit to kdrag0n/pyrobud that referenced this issue Dec 15, 2019
This helps accomodate the growing Bot class, since it's easily divided
into several logical components anyway. Each component is an ABC
(Abstract Base Class) that functions as a mixin and thus can access all
the other attributes of the Bot class. Bot subclasses all the mixins to
unify them. Telethon uses the same approach to divide TelegramClient
into many mixins that each contain different categories of API methods.

An adaptive MixinBase reference that is defined as abc.ABC during
runtime and Bot during type checking is used as the subclass for each
mixin to work around python/mypy#5837.
Furthermore, the MixinBase reference itself has its type specified as
Any to work around python/mypy#2477.

The only change for outsiders should be the Bot class being moved to the
`core` module.

Signed-off-by: Danny Lin <danny@kdrag0n.dev>
@micahjsmith
Copy link

For anyone who comes across this via the original report, a simple way to address sqlalchemy/mypy issues is to also import DeclarativeMeta.

from sqalalchemy.ext.declarative import declarative_base, DeclarativeMeta
Base: DeclarativeMeta = declarative_base()
...

@kevlarr
Copy link

kevlarr commented Dec 9, 2020

Thanks @micahjsmith, just ran into this for the first time (on a 2-yo project, no clue why it was never reporting before). Base: Declarative = ... is way better than Base = ... # type: ignore

palubaj added a commit to palubaj/sapcli that referenced this issue May 2, 2023
Mypy struggles with dynamic type creation.
See: python/mypy#2477
palubaj added a commit to palubaj/sapcli that referenced this issue May 2, 2023
Mypy struggles with dynamic type creation.
See: python/mypy#2477
palubaj added a commit to palubaj/sapcli that referenced this issue May 9, 2023
Mypy struggles with dynamic type creation.
See: python/mypy#2477
palubaj added a commit to palubaj/sapcli that referenced this issue May 9, 2023
Mypy struggles with dynamic type creation.
See: python/mypy#2477
palubaj added a commit to palubaj/sapcli that referenced this issue May 11, 2023
Mypy struggles with dynamic type creation.
See: python/mypy#2477
jfilak pushed a commit to jfilak/sapcli that referenced this issue May 11, 2023
Mypy struggles with dynamic type creation.
See: python/mypy#2477
johnnyjly referenced this issue in csc301-2023-fall/project-34-qeynet-inc-t Dec 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests