Python method overriding - more specific arguments in derived than base class
Asked Answered
P

2

6

Let's say I want to create an abstract base class called Document. I want the type checker to guarantee that all its subclasses implement a class method called from_paragraphs, which constructs a document from a sequence of Paragraph objects. However, a LegalDocument should only be constructable from LegalParagraph objects, and an AcademicDocument - only from AcademicParagraph objects.

My instinct is to do it like so:

from abc import ABC, abstractmethod
from typing import Sequence


class Document(ABC):
    @classmethod
    @abstractmethod
    def from_paragraphs(cls, paragraphs: Sequence["Paragraph"]):
        pass


class LegalDocument(Document):
    @classmethod
    def from_paragraphs(cls, paragraphs: Sequence["LegalParagraph"]):
        return  # some logic here...


class AcademicDocument(Document):
    @classmethod
    def from_paragraphs(cls, paragraphs: Sequence["AcademicParagraph"]):
        return  # some logic here...


class Paragraph:
    text: str


class LegalParagraph(Paragraph):
    pass


class AcademicParagraph(Paragraph):
    pass

However, Pyright complains about this because from_paragraphs on the derived classes violates the Liskov substitution principle. How do I make sure that each derived class implements from_paragraphs for some kind of Paragraph?

Plot answered 26/1 at 18:48 Comment(8)
If the child classes must implement from_paragraphs(), why does the base class implement that method at all? Couldn't you just omit that method from the base class?Rowland
Not sure this entirely works, but Document.from_paragraphs should be generic. Document.from_paragraphs doesn't need to work with a list of arbitrary mix of Paragraph subclasses; rather the overrides each need to work with a homogenous list of a particular subtype of Paragraph.Intricacy
@JohnGordon I'll edit my question to make this clearer: I want the child classes to be forced to implement this method for some kind of Paragraph, so that the type checker approves of some other function which takes a Document and uses its .from_paragraph method. I maybe made this too confusing by using classmethods but I'll keep them in the example to not derail the discussion.Plot
@Intricacy yeah! I think I'm probably looking for some solution with generics, but I'm not sure how to approach it exactly.Plot
I had played around with it for a while, and there's still an issue because, e.g., Sequence["LegalParagraph"] is still a narrowing of Sequence[P] for inheritance purposes. I think applying LSP to class methods is already a bit dicey, since although you can call a class method using an instance (it just passes the class of the instance, not the instance itself as the implicit argument), the behavior of the method typically is independent of the that instance. (1/2)Intricacy
All of which is to say, if I were writing a similar question, I would be asking if mypy is correct in flagging this as an LSP violation in the first place, because now I am curious. (2/2)Intricacy
@Intricacy the error I get is not actually "LSP violation", it's Method "from_paragraphs" overrides class "Document" in an incompatible manner (...and I use PyRight ;) ). Either way, you could have some function accept type[Document] and then expect to be able to construct that type from_paragraphs, so I suppose this makes sense to call an LSP violation 🤷Plot
What I suspect is that type checkers need some support for associated types, where Document and its subclasses all have, say, a class attribute Paragraph that's assigned the appropriate Paragraph subtype, and then you can reference that using some sort of syntax like def from_paragraphs(cls: Type[T], paragraphs: List[T.Paragraph]). T here would be something like TypeVar('T', bound=Document).Intricacy
P
0

Turns out this can be solved using generics:

from abc import ABC, abstractmethod
from typing import Generic, Sequence, TypeVar

ParagraphType = TypeVar("ParagraphType", bound="Paragraph")


class Document(ABC, Generic[ParagraphType]):
    @classmethod
    @abstractmethod
    def from_paragraphs(cls, paragraphs: Sequence[ParagraphType]):
        pass


class LegalDocument(Document["LegalParagraph"]):
    @classmethod
    def from_paragraphs(cls, paragraphs):
        return  # some logic here...


class AcademicDocument(Document["AcademicParagraph"]):
    @classmethod
    def from_paragraphs(cls, paragraphs):
        return  # some logic here...


class Paragraph:
    text: str


class LegalParagraph(Paragraph):
    pass


class AcademicParagraph(Paragraph):
    pass

Saying bound="Paragraph" guarantees that the ParagraphType represents a (subclass of) Paragraph, but the derived classes are not expected to implement from_paragraphs for all paragraph types, just for the one they choose. The type checker also automatically figures out the type of the argument paragraphs for LegalDocument.from_paragraphs, saving me some work :)

Plot answered 27/1 at 22:16 Comment(0)
F
0

This pattern is called factory pattern: depends of the input, you get different types of object. What you have there will not work because:

# Because Document should not have knowledge of derived class:
doc = Document.from_paragraphs(...)

# Because type mismatch
doc = LegalDocument.from_paragraphs([AcademicParagraphs()]) 

Here is how I approach this problem:

class Document:
    def __init__(self, paragraphs):
        self.paragraphs = paragraphs

class LegalDocument(Document):
    pass

class AcademicDocument(Document):
    pass

class Paragraph:
    def __init__(self, text):
        self.text = text

class LegalParagraph(Paragraph):
    pass

class AcademicParagraph(Paragraph):
    pass

def create_document(*paragraphs):
    # assume that all paragraphs are of the same type
    if isinstance(paragraphs[0], LegalParagraph):
        klass = LegalDocument
    elif isinstance(paragraphs[0], AcademicParagraph):
        klass = AcademicDocument
    else:
        raise TypeError()

    return klass(paragraphs)


d1 = create_document(LegalParagraph("foo"), LegalParagraph("bar"))
assert isinstance(d1, LegalDocument)

d2 = create_document(AcademicParagraph("moo"))
assert isinstance(d2, AcademicDocument)

Notes

  • I will have a simple set of classes, not messing around with ABC
  • No class methods, the __ini__() will be enough
  • I have a single factory function create_document, which will create a document where the type depends on the input
  • A different approach is to tweak Document.__new__() method, but that requires some knowledge of how __new__() works and not everybody knows that.
Frigging answered 26/1 at 19:28 Comment(1)
Thanks @Hai :) The thing is, I don't actually need the subclass of Document dynamically determined for me - what I want is type safety: as the creator of Document, I want to be sure that all derived classes can be constructed from a sequence of some kind of Paragraph objects.Plot
P
0

Turns out this can be solved using generics:

from abc import ABC, abstractmethod
from typing import Generic, Sequence, TypeVar

ParagraphType = TypeVar("ParagraphType", bound="Paragraph")


class Document(ABC, Generic[ParagraphType]):
    @classmethod
    @abstractmethod
    def from_paragraphs(cls, paragraphs: Sequence[ParagraphType]):
        pass


class LegalDocument(Document["LegalParagraph"]):
    @classmethod
    def from_paragraphs(cls, paragraphs):
        return  # some logic here...


class AcademicDocument(Document["AcademicParagraph"]):
    @classmethod
    def from_paragraphs(cls, paragraphs):
        return  # some logic here...


class Paragraph:
    text: str


class LegalParagraph(Paragraph):
    pass


class AcademicParagraph(Paragraph):
    pass

Saying bound="Paragraph" guarantees that the ParagraphType represents a (subclass of) Paragraph, but the derived classes are not expected to implement from_paragraphs for all paragraph types, just for the one they choose. The type checker also automatically figures out the type of the argument paragraphs for LegalDocument.from_paragraphs, saving me some work :)

Plot answered 27/1 at 22:16 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.