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

Documentation for constructing abstract base classes is misleading #90970

Closed
Yoshanuikabundi mannequin opened this issue Feb 21, 2022 · 5 comments
Closed

Documentation for constructing abstract base classes is misleading #90970

Yoshanuikabundi mannequin opened this issue Feb 21, 2022 · 5 comments
Labels
3.11 only security fixes docs Documentation in the Doc dir

Comments

@Yoshanuikabundi
Copy link
Mannequin

Yoshanuikabundi mannequin commented Feb 21, 2022

BPO 46814
Nosy @gvanrossum, @rhettinger, @Yoshanuikabundi
PRs
  • bpo-46814: Clarify requirements of abstract base classes #31463
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2022-02-21.08:25:45.315>
    labels = ['3.11', 'docs']
    title = 'Documentation for constructing abstract base classes is misleading'
    updated_at = <Date 2022-02-28.16:58:45.799>
    user = 'https://github.com/Yoshanuikabundi'

    bugs.python.org fields:

    activity = <Date 2022-02-28.16:58:45.799>
    actor = 'gvanrossum'
    assignee = 'docs@python'
    closed = False
    closed_date = None
    closer = None
    components = ['Documentation']
    creation = <Date 2022-02-21.08:25:45.315>
    creator = 'Yoshanuikabundi'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 46814
    keywords = ['patch']
    message_count = 5.0
    messages = ['413639', '413682', '413683', '413686', '414208']
    nosy_count = 4.0
    nosy_names = ['gvanrossum', 'rhettinger', 'docs@python', 'Yoshanuikabundi']
    pr_nums = ['31463']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = None
    url = 'https://bugs.python.org/issue46814'
    versions = ['Python 3.11']

    @Yoshanuikabundi
    Copy link
    Mannequin Author

    Yoshanuikabundi mannequin commented Feb 21, 2022

    The docs for the abc[0] module states "With this class, an abstract base class can be created by simply deriving from ABC", and then gives an example of a class with no contents. This is not sufficient to construct an ABC; an ABC in Python additionally requires at least one abstract method. This can be demonstrated by executing the example code and instantiating it (ABCs cannot be instantiated) or calling the inspect.isabstract() function on it (returns False). The requirement is also (cryptically) explicated in the Implementation paragraph of the "The abc Module: an ABC Support Framework" section of PEP-3119[1]. This requirement of implementing an abstract method is not mentioned in the docs for the abc module or in the module's docstrings. An ABC with no abstract methods is sometimes used to mark a parent class that is not intended to be instantiated on its own, so this limitation of the Python implementation should be documented.

    [0] https://docs.python.org/3.11/library/abc.html

    [1] https://www.python.org/dev/peps/pep-3119/#the-abc-module-an-abc-support-framework

    @Yoshanuikabundi Yoshanuikabundi mannequin added the 3.11 only security fixes label Feb 21, 2022
    @Yoshanuikabundi Yoshanuikabundi mannequin added docs Documentation in the Doc dir 3.11 only security fixes labels Feb 21, 2022
    @Yoshanuikabundi Yoshanuikabundi mannequin added the docs Documentation in the Doc dir label Feb 21, 2022
    @rhettinger
    Copy link
    Contributor

    To me, this looks like a way too extensive edit jsut to emphasize a corner case that rarely arises in practice. It bends over backwards to force an awkward definition regarding what an ABC really is.

    A more minimal edit is to just note that inspect.isabstract() returns true if a class inherits from ABC and has at least one remaining abstractmethod.

    That tells the truth without having to redefine an ABC. And it matches the spirit of the ABC mechanism. At no point does ABCMeta ever require that any class in the chain must have ever defined an abstractmethod. Instead, its only rule is that if there is least one remaining abstractmethod, it will refuse to instantiate. Roughly:

      if any(getattr(meth, '__isabstractmethod__', False) for meth in meths):
         raise TypeError("Can't instantiate")
      instantiate(cls)

    Note, any() defaults to False if there are no inputs and that the actual C code works the same way. Even if a class never defines any abstractmethods, this test still occurs. For an empty ABC, it just happens to always succeed in instantiating because there is no work left to be done.

    Worldviews aside, I don't think it helpful to users to everywhere rewrite what it means to be an ABC just to make it match the output of inspect.isabstract() which is just short for inspect.has_abstract_methods_remaining().

    Guido, any thoughts?

    @rhettinger
    Copy link
    Contributor

    An analogy may help. Release managers must check the list of release blockers and stop if the list is non-empty. If no release blockers were ever filed, the release blockers list is empty, but it still exists and its definition hasn't changed.

    The test is_blocked(blockers) tells us whether the list is non-empty. It doesn't redefine what it means to be a blockers list.

    @AlexWaygood AlexWaygood changed the title Documentation for constructin abstract base classes is misleading Documentation for constructing abstract base classes is misleading Feb 21, 2022
    @AlexWaygood AlexWaygood changed the title Documentation for constructin abstract base classes is misleading Documentation for constructing abstract base classes is misleading Feb 21, 2022
    @rhettinger
    Copy link
    Contributor

    FWIW, I’m only -0 on this. It is also perfectly reasonable to say that a class is abstract if and only if there is at least one remaining abstract method. After 15 years though, I’m inclined to say that the status quo wins.

    @gvanrossum
    Copy link
    Member

    Raymond, I agree that this is going too far. I believe the OP has taken the position that "abstract" has only one meaning and it is defined by inspect.isabstract(). I disagree with this.

    An ABC is an ABC is an ABC, and it provides certain functionality through the ABCMeta metaclass: (1) forbid instantiation when at least one @abstractmethod-decorated method exist that isn't overridden, and (2) virtual subclasses.

    Calling out that a class with metaclass=ABCMeta is only abstract when it has at least one @AbstractMethod left, over and over, is not helpful.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes docs Documentation in the Doc dir
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants