Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@bentheiii

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

I have made the requested changes; please review again

Thanks for making the requested changes!

@ericvsmith: please review the changes made to this pull request.

Since update_abstractmethods is documented as being usable as a decorator, there should be a test for that. I realize it's trivially obvious that it works, but still. Although I guess you'll need another decorator that adds an abstract method (like dataclasses used to do) in order to test it. And then use them together.

Since update_abstractmethods is documented as being usable as a decorator, there should be a test for that. I realize it's trivially obvious that it works, but still. Although I guess you'll need another decorator that adds an abstract method (like dataclasses used to do) in order to test it. And then use them together.

Change looks good. Thanks.

My only real concern is that dataclasses now imports abc. We've jumped through hoops to make dataclasses not import typing, because it's slow to import. I don't know if abc brings the same issues, but I'll try to do some investigating. The trick it uses for typing is to check if 'typing' is in sys.modules before checking for typing.<somthing_or_other>. I'm not sure such trickery is warranted here. Probably not, since just starting the REPL looks like it imports abc (but not typing).

If this does prove to be an issue, then we can change the isinstance(cls, ABCMeta) checking to hasattr(cls, '__abstractmethods__') (no sys.modules check necessary), and then import abc.update_abstractmethods before calling it inside the "if" block.

Just noted this:

@ericvsmith

My only real concern is that dataclasses now imports abc.

I looked into this, dataclasses already imports functools which already imports abc, so there's no new module being loaded. Also, abc.py is pretty compact and backed by C code -- most of the Python code is in _py_abc.py which is only loaded if _abc (the C code) cannot be loaded for some reason.

So I don't think there's a worry. @ericvsmith, all your other comments seem to have been taken care of this -- can you sign off? (Unless you want to jump into the debate about subclasses.)

@gvanrossum: Agreed about importing abc. I'll let you and the others work out the subclass issue.

Of course, I defer judgement to the dev team, but I'd be remiss if I didn't reflect the implications of this behavior:

When this behavior is implemented, it will effectively split all class decorators into one of three categories:

1. ABC-agnostic, who don't consider abstract superclasses as a use case. total_ordering will be of this category.
2. ABC-aware, subclass-agnostic, who accept the possibility of an abstract superclass, and thus call update_abstractmethods. dataclass will be of this of this category, as well as, update_abstractmethods, techinally.
3. ABC-aware, subclass-aware, who accept the possibility of an abstract superclass, and call update_abstractmethods, and also accept possibility of it having subclasses, so they also call update_abstractmethods on its subclasses (perhaps even recursively).

An important consequence is, that decorators can only be reliably stacked on top of each other only if they are of the same category. And while update_abstractmethods can be used to adapt a category 1 decorator to category 2, there are no simple means to adapt to category 3.

Of course, the mere existence of a decorator of the third category is a case so far fetched that I doubt it even exists across all python code. This is a valid reason to discount it, but the implication is that if were to ever exist, it could not be used with virtually any other decorator.

@rhettinger any final word? @iritkatriel any comments?

@rhettinger any final word? @iritkatriel any comments?

A very pedantic one, which should not block this PR being merged. The doc says "It does not update any subclasses", but no test asserts this.

Thanks! All merged.