Skip to content

Handling mixins that reference fields that they do not contain #5837

Closed

Description

@takeda

This is more of a question or a feature request if there's not a good way to do it.

I'm currently adding types to existing python 2.7 code base hoping that it will help me with migrating the code to Python 3.

I run into code that looks like this:

#! /usr/bin/env python

class A(object):
	def __init__(self, var1, var2):
		# type: (str, str) -> None
		self.var1 = var1
		self.var2 = var2

class Mixin(object):
	def funcA(self):
		# type: () -> str
		return self.var1

class B(A, Mixin):
	pass

def main():
	# type: () -> None

	a = B('value1', 'value2')
	print(a.funcA())

if __name__ == '__main__':
	main()

This is valid Python code, i.e.:

$ ./test.py
value1

But mypy is complaining:

$ ../py37/bin/mypy --python-executable ../py27/bin/python test.py
test.py:12: error: "Mixin" has no attribute "var1"

I understand the issue, there's nothing in Mixin class that states that var1 will be available.

My question is is there a way to tell mypy somehow that this attribute will be available?

I did not have much luck with using abstract classes and Protocol.
I also had a suggestion to use stub file, but that doesn't look like be perfect solution either, since I might need to define types outside of the module and keep them in sync. And this pattern is used in multiple places not just one.

Is there a way how this could be expressed in mypy? If the code needs to be slightly refactored without changing how it works I suppose that might also be acceptable.

Activity

  1. msullivan commented on Oct 25, 2018

    @msullivan
    Collaborator

    How about (for python 2.7; in python 3.6 or greater you'd use a variable type annotation):

    MYPY = False
    class Mixin(object):
        if MYPY:
            var1 = None  # type: str
    
        def funcA(self):
            # type: () -> str
            return self.var1
    

    (The if MYPY stuff isn't strictly necessary, but it keeps the runtime behavior from changing.)

    I had hoped it would work to declare var1 as an abstractproperty of Mixin, but mypy doesn't seem able to figure out they are compatible.

  2. takeda commented on Oct 25, 2018

    @takeda
    Author

    That actually will work, and probably will do it this way, but if there was a way to define some parent class that only mypy would see would be perfect. So I would need to define these fields only once.

  3. JelleZijlstra commented on Oct 25, 2018

    @JelleZijlstra
    Member

    You can probably do that with something like:

    if MYPY:
        _Base = SomeRealClass
    else:
        _Base = object
    
    class Mixin(_Base): ...
    
  4. takeda commented on Oct 26, 2018

    @takeda
    Author

    That actually seems to work rather well, at least for the code so far. I used TYPE_CHECKING from typing instead of MYPY, is there any disadvantage to use that instead?

  5. gvanrossum commented on Oct 26, 2018

    @gvanrossum
    Member

    You should use typing.TYPE_CHECKING. Using MYPY is only needed when you are stuck with a very old version of typing.py.

  6. takeda commented on Oct 26, 2018

    @takeda
    Author

    Awesome, thank you all for your help.

  7. reopened this on Oct 26, 2018
  8. takeda commented on Oct 26, 2018

    @takeda
    Author

    Actually looks like this still has an issue. When I'm now checking for classes that includes mixins I'm getting error similar to this: test.py:21: error: Cannot determine consistent method resolution order (MRO) for "B"

  9. gvanrossum commented on Oct 26, 2018

    @gvanrossum
    Member
  10. takeda commented on Oct 26, 2018

    @takeda
    Author

    This is how the code looks now after the change:

    #! /usr/bin/env python
    
    from typing import TYPE_CHECKING
    
    class A(object):
    	def __init__(self, var1, var2):
    		# type: (str, str) -> None
    		self.var1 = var1
    		self.var2 = var2
    
    if TYPE_CHECKING:
    	_Base = A
    else:
    	_Base = object
    
    class Mixin(_Base):
    	def funcA(self):
    		# type: () -> str
    		return self.var1
    
    class B(A, Mixin):
    	pass
    
    def main():
    	# type: () -> None
    
    	a = B('value1', 'value2')
    	print(a.funcA())
    
    if __name__ == '__main__':
    	main()
  11. gvanrossum commented on Oct 26, 2018

    @gvanrossum
    Member
  12. takeda commented on Oct 26, 2018

    @takeda
    Author

    Yeah, if I place mixin first then things work and I no longer even need to make type checking a special case, just have the base class a parent of mixin by default. Seems like that is the correct way of using mixins. My only concern is that this is an existing code base and I'm wondering if this could break something this way.

  13. gvanrossum commented on Oct 26, 2018

    @gvanrossum
    Member

    I don't think we can help you with that...

  14. leonardoramirezr commented on Jul 9, 2020

    @leonardoramirezr

    Try with:

    from typing import Type, TYPE_CHECKING, TypeVar
    
    T = TypeVar('T')
    
    
    def with_typehint(baseclass: Type[T]) -> Type[T]:
        """
        Useful function to make mixins with baseclass typehint
    
        ```
        class ReadonlyMixin(with_typehint(BaseAdmin))):
            ...
        ```
        """
        if TYPE_CHECKING:
            return baseclass
        return object

    Example:

    class ReadOnlyInlineMixin(with_typehint(BaseModelAdmin)):
        def get_readonly_fields(self,
                                request: WSGIRequest,
                                obj: Optional[Model] = None) -> List[str]:
    
            if self.readonly_fields is None:
                readonly_fields = []
            else:
                readonly_fields = self.readonly_fields # self get is typed by baseclass
    
            return self._get_readonly_fields(request, obj) + list(readonly_fields)
    
        def has_change_permission(self,
                                  request: WSGIRequest,
                                  obj: Optional[Model] = None) -> bool:
            return (
                request.method in ['GET', 'HEAD']
                and super().has_change_permission(request, obj) # super is typed by baseclass
            )
    
    >>> ReadOnlyAdminMixin.__mro__
    (<class 'custom.django.admin.mixins.ReadOnlyAdminMixin'>, <class 'object'>)
  15. jaredlockhart commented on Jul 21, 2020

    @jaredlockhart

    @leonardon473 trying your proposal results in:

    Unsupported dynamic base class "with_typehint" mypy(error)

    using:
    Python 3.8.2
    mypy==0.770

    The closest reference I find to that error is here:

    #6372

    which suggests that mypy does not accept dynamically generated base classes?

    Also mypy complained that return object is an unreachable statement.

  16. jsatt commented on Jul 28, 2020

    @jsatt

    An alternate solution for future folks which this doesn't work:

    I was having a similar problem making a QuerySet Mixin for Django.

    class Mixin:
        def do_stuff(self, value: str) -> QuerySet:
            return self.filter(abc=value)

    Resulted in "Mixin" has no attribute "filter"

    So I tried the solution above:

    if TYPE_CHECKING:
        _Base = QuerySet
    else:
        _Base = object
    
    
    class Mixin(_Base):
        def do_stuff(self, value: str) -> QuerySet:
            return self.filter(abc=value)

    And got Invalid base class "_Base"

    Not sure why it's failing with QuerySet, but also with Model and Field based classes. Most other classes seem to work fine, just not those (maybe something in the metaclasses Django uses?).

    I finally got it to work by type hinting self,

    class Mixin:
        def do_stuff(self: QuerySet, value: str) -> QuerySet:
            return self.filter(abc=value)

    If anyone has feedback as to why the other ways aren't working I'd be happy to hear about it (CPYthon 3.8.0, Mypy: 0.780), but this solution worked and is also more succinct that the others.

  17. BakinSergey commented on Aug 18, 2023

    @BakinSergey

    choose 1. u can specify needed field with their typehints as classvariables of Mixin class. It's will be more pythonic(more explicit)
    choose 2. u can inherit protocol (Typing.Protocol) of class which Mixin extended,
    but this will not work if master class of mixin class inherit sqlalchemy Model :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions