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

Able to instantiate a subclass with abstract methods from __init_subclass__ of the ABC #79996

Closed
jbasko mannequin opened this issue Jan 24, 2019 · 12 comments
Closed

Able to instantiate a subclass with abstract methods from __init_subclass__ of the ABC #79996

jbasko mannequin opened this issue Jan 24, 2019 · 12 comments
Assignees
Labels
3.8 only security fixes 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error

Comments

@jbasko
Copy link
Mannequin

jbasko mannequin commented Jan 24, 2019

BPO 35815
Nosy @gvanrossum, @rhettinger, @ethanfurman, @isidentical, @jbasko, @akulakov

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 = 'https://github.com/gvanrossum'
closed_at = <Date 2021-07-09.02:42:08.967>
created_at = <Date 2019-01-24.07:57:05.166>
labels = ['type-bug', '3.8', '3.9', '3.10', '3.11', 'invalid']
title = 'Able to instantiate a subclass with abstract methods from __init_subclass__ of the ABC'
updated_at = <Date 2021-07-09.02:42:08.967>
user = 'https://github.com/jbasko'

bugs.python.org fields:

activity = <Date 2021-07-09.02:42:08.967>
actor = 'gvanrossum'
assignee = 'gvanrossum'
closed = True
closed_date = <Date 2021-07-09.02:42:08.967>
closer = 'gvanrossum'
components = []
creation = <Date 2019-01-24.07:57:05.166>
creator = 'jbasko'
dependencies = []
files = []
hgrepos = []
issue_num = 35815
keywords = []
message_count = 12.0
messages = ['334284', '365439', '383700', '383947', '386101', '397142', '397173', '397174', '397175', '397176', '397177', '397182']
nosy_count = 8.0
nosy_names = ['gvanrossum', 'rhettinger', 'stutzbach', 'ethan.furman', 'BTaskaya', 'jbasko', 'tda', 'andrei.avk']
pr_nums = []
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue35815'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10', 'Python 3.11']

@jbasko
Copy link
Mannequin Author

jbasko mannequin commented Jan 24, 2019

I am creating and registering singleton instances of subclasses of ABC in the ABC's __init_subclass__ and I just noticed that I am able to instantiate even the classes which have abstract methods.

import abc


class Base(abc.ABC):
    def __init_subclass__(cls, **kwargs):
        instance = cls()
        print(f"Created instance of {cls} easily: {instance}")


    @abc.abstractmethod
    def do_something(self):
        pass


class Derived(Base):
    pass

Actual Output:

Created instance of <class '__main__.Derived'> easily: <main.Derived object at 0x10a6dd6a0>

Expected Output:

TypeError: Can't instantiate abstract class Derived with abstract methods do_something

@jbasko jbasko mannequin added 3.7 (EOL) end of life type-bug An unexpected behavior, bug, or error labels Jan 24, 2019
@isidentical
Copy link
Sponsor Member

__init_subclass__ is called way before (in the type_new, when type is in the process of getting created) the object's __new__ (object_new) (which raises that TypeError).

@tda tda mannequin added the 3.8 only security fixes label Sep 9, 2020
@ethanfurman
Copy link
Member

I tried update abc.py with the same dance I have to use with Enum:

    def __new__(mcls, name, bases, namespace, **kwargs):
        # remove current __init_subclass__ so previous one can be found with getattr
        try:
            new_init_subclass = namespace.get('__init_subclass__')
            del namespace['__init_subclass__']
        except KeyError:
            pass
        # create our new ABC type
        if bases:
            bases = (_NoInitSubclass, ) + bases
            abc_cls = super().__new__(mcls, name, bases, namespace, **kwargs)
            abc_cls.__bases__ = abc_cls.__bases__[1:]
        else:
            abc_cls = super().__new__(mcls, name, bases, namespace, **kwargs)
        old_init_subclass = getattr(abc_cls, '__init_subclass__', None)
        # restore new __init_subclass__ (if there was one)
        if new_init_subclass is not None:
            abc_cls.__init_subclass__ = classmethod(new_init_subclass)
        _abc_init(abc_cls)
        # call parents' __init_subclass__
        if old_init_subclass is not None:
            old_init_subclass(**kwargs)
        return abc_cls

But I get this error:

Fatal Python error: init_import_site: Failed to import the site module
Python runtime state: initialized
Traceback (most recent call last):
  File "/home/ethan/source/python/cpython/Lib/site.py", line 73, in <module>
    import os
  File "/home/ethan/source/python/cpython/Lib/os.py", line 29, in <module>
    from _collections_abc import _check_methods
  File "/home/ethan/source/python/cpython/Lib/_collections_abc.py", line 122, in <module>
    class Coroutine(Awaitable):
  File "/home/ethan/source/python/cpython/Lib/abc.py", line 103, in __new__
    abc_cls.__bases__ = abc_cls.__bases__[1:]
TypeError: __bases__ assignment: 'Awaitable' object layout differs from '_NoInitSubclass'

@ethanfurman
Copy link
Member

If the patch in bpo-42775 is committed, this problem will be solved.

@ethanfurman ethanfurman self-assigned this Dec 29, 2020
@ethanfurman ethanfurman added 3.10 only security fixes and removed 3.7 (EOL) end of life 3.8 only security fixes labels Feb 1, 2021
@ethanfurman ethanfurman removed their assignment Feb 1, 2021
@ethanfurman
Copy link
Member

That patch was rejected in favor of updating Enum to use __set_name__ to do the final creation of enum members.

The same thing could be done for ABC, but I lack the C skills to make it happen.

@akulakov
Copy link
Contributor

akulakov commented Jul 8, 2021

Ethan: as far as I understand, there's no actual problem here. The report adds a print statement in __init_subclass__ that says that instance was created, while in fact instance is not created. I can confirm what Batuhan said, in my testing, that it fails with TypeError as expected:

python3 ~/temp/a.py ----VICMD----

Created instance of <class '__main__.Derived'> easily: <__main__.Derived object at 0x106fbb9d0>
Traceback (most recent call last):
  File "/Users/ak/temp/a.py", line 54, in <module>
    Derived()
TypeError: Can't instantiate abstract class Derived with abstract method do_something

I think this can be closed as not a bug.

@ethanfurman
Copy link
Member

Andrei, which code did you try? I'm still seeing the failure (i.e. the class being created) in 3.8 and forward.

@ethanfurman ethanfurman added 3.8 only security fixes 3.9 only security fixes 3.11 only security fixes labels Jul 9, 2021
@akulakov
Copy link
Contributor

akulakov commented Jul 9, 2021

Ethan, here is the code I tried with 3.9, and the failure:

import abc
class Base(abc.ABC):
    def __init_subclass__(cls, **kwargs):
        instance = cls()
        print(f"Created instance of {cls} easily: {instance}")
    @abc.abstractmethod
    def do_something(self):
        pass
class Derived(Base):
    pass
Derived()

Output:
python3 ~/temp/a.py ----VICMD----

Traceback (most recent call last):
  File "/Users/ak/temp/a.py", line 53, in <module>
    Derived()
TypeError: Can't instantiate abstract class Derived with abstract method do_something

@akulakov
Copy link
Contributor

akulakov commented Jul 9, 2021

I missed the fact that instance was being indeed created, I can see it now.

@akulakov
Copy link
Contributor

akulakov commented Jul 9, 2021

Oops, I see the issue now. I was playing around with the code sample and had the print line commented out inadvertently when I made the output I pasted above. Sorry for the noise!

@rhettinger
Copy link
Contributor

At worst, this seems like only a minor nuisance. The ABC metaclass is limited in its powers and seems to reasonably cover the common use cases.
I recommend leaving it as is. Guido, what do you think?

@gvanrossum
Copy link
Member

At worst, this seems like only a minor nuisance. The ABC metaclass is limited in its powers and seems to reasonably cover the common use cases.
I recommend leaving it as is. Guido, what do you think?

Agreed. The abstractness checks are limited and not intended to prevent all ways of creating abstract instances -- just to catch typical mistakes early. (I vaguely recall another bug report about a weakness in this check that I resolved in the same way.)

@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.8 only security fixes 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

5 participants