classification
Title: ABCMeta.register() should work as a decorator
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.3
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.araujo Nosy List: benjamin.peterson, daniel.urban, eric.araujo, gvanrossum, kerio, pitrou, rhettinger, stutzbach
Priority: normal Keywords: patch

Created on 2011-01-08 23:32 by kerio, last changed 2011-02-26 16:32 by stutzbach. This issue is now closed.

Files
File name Uploaded Description Edit
abcmeta_register_deco.diff kerio, 2011-01-08 23:32 proposed patch
abcmeta_register_deco_with_docs.diff kerio, 2011-01-09 00:05 proposed patch with docs and corrections
abcmeta_register_v3.diff eric.araujo, 2011-01-09 00:19
abcmeta_register_v4.diff kerio, 2011-01-10 17:51
abcmeta_register_v4_plus_tests.diff kerio, 2011-01-10 19:25
Messages (18)
msg125804 - (view) Author: Edoardo Spadolini (kerio) Date: 2011-01-08 23:32
If we make ABCMeta.register() return the registered class, like atexit.register() does for the registered function, we can then use it as a decorator:

Now:

class Foo:
    ...
ABarC.register(Foo)

With this change:

@ABarC.register
class Foo:
    ...

The only problem this would cause is in code that relies on ABCMeta.register() to return None; I can't think of a reason anyone would rely on this, but...
msg125806 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2011-01-08 23:33
Looks like a good idea to me.
msg125807 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-01-08 23:41
Thanks for the report and patch.  Some comments:


+        """Register a virtual subclass of an ABC. Returns the said subclass."""
Should be “Return”, for consistency.

+        return subclass # For usage as a decorator
I’d move that to the docstring.

Can you edit the docs (remember to use the versionchanged directive) too?
msg125810 - (view) Author: Edoardo Spadolini (kerio) Date: 2011-01-09 00:05
Ok, edited the docs too; hope I did everything right...
msg125814 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-01-09 00:19
Edited your patch to fix some nits.  If there is no opposition, I’ll commit this to py3k when 3.2 is out.
msg125816 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2011-01-09 00:30
+1
msg125905 - (view) Author: Daniel Urban (daniel.urban) * Date: 2011-01-10 17:36
There is another return statement earlier in the ABCMeta.register method. The patch probably should also modify that.
msg125907 - (view) Author: Edoardo Spadolini (kerio) Date: 2011-01-10 17:51
Whoops, corrected that - should I add some more tests for that too?
msg125909 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-01-10 17:57
You should, otherwise how would you prove it’s fixed? :)
msg125912 - (view) Author: Edoardo Spadolini (kerio) Date: 2011-01-10 18:58
Fair enough :)
msg125915 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-01-10 19:33
-            return  # Already a subclass
+            return subclass # Already a subclass
PEP 8 advises to put two spaces before inline comments, for readability.
 (There is no need to upload a new patch, I’ll change that before commit.)

+        self.assertIsInstahce(c, (A,))
This is fixed in the newest version of your patch.  Running tests before
uploading diffs catches such errors :)
msg126003 - (view) Author: Edoardo Spadolini (kerio) Date: 2011-01-11 12:41
Yeah, I should've waited for the test to finish, but come on, "it was just a small change" :(

Now I know why you should always test everything at least, sorry about that :)
msg129298 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-02-24 18:18
Committed to py3k as r88545.  You’ll notice that I fixed the nesting of the versionchanged directive and that I changed my mind about “returns”.  Thanks again!
msg129300 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2011-02-24 18:37
In what use-cases would you want to call MyABC.register() when defining a class instead of inheriting from MyABC?

I always thought of the register() as hack to make it possible to support types written in C, which can't inherit from the ABC.
msg129310 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2011-02-24 21:20
Someone may want to register with an ABC but not inherit methods or add a class to the mro.  It’s always been allowed by the register method; the new decorator feature is just a very minor nicety on top of that.

Edoardo, was your request motivated by a real use case where you didn’t want to inherit from the ABC?
msg129521 - (view) Author: Edoardo Spadolini (kerio) Date: 2011-02-26 10:35
Not really, but putting something in your inheritance lattice only to mark a class as providing an interface just doesn't seem right to me.
msg129536 - (view) Author: Daniel Urban (daniel.urban) * Date: 2011-02-26 12:32
> In what use-cases would you want to call MyABC.register() when defining
> a class instead of inheriting from MyABC?

For example if you don't want to inherit a __dict__ for a tree-like data structure (see also issue11333).
msg129571 - (view) Author: Daniel Stutzbach (stutzbach) (Python committer) Date: 2011-02-26 16:32
Sounds good to me.  Thanks for the clarifications and satisfying my curiosity! :-)
History
Date User Action Args
2011-02-26 16:32:09stutzbachsetnosy: gvanrossum, rhettinger, pitrou, benjamin.peterson, stutzbach, eric.araujo, daniel.urban, kerio
messages: + msg129571
2011-02-26 12:32:16daniel.urbansetnosy: gvanrossum, rhettinger, pitrou, benjamin.peterson, stutzbach, eric.araujo, daniel.urban, kerio
messages: + msg129536
2011-02-26 10:35:39keriosetnosy: gvanrossum, rhettinger, pitrou, benjamin.peterson, stutzbach, eric.araujo, daniel.urban, kerio
messages: + msg129521
2011-02-24 21:20:17eric.araujosetnosy: gvanrossum, rhettinger, pitrou, benjamin.peterson, stutzbach, eric.araujo, daniel.urban, kerio
messages: + msg129310
2011-02-24 18:37:24stutzbachsetnosy: + stutzbach
messages: + msg129300
2011-02-24 18:18:29eric.araujosetstatus: open -> closed
nosy: gvanrossum, rhettinger, pitrou, benjamin.peterson, eric.araujo, daniel.urban, kerio
messages: + msg129298

resolution: fixed
stage: patch review -> resolved
2011-01-11 12:41:07keriosetnosy: gvanrossum, rhettinger, pitrou, benjamin.peterson, eric.araujo, daniel.urban, kerio
messages: + msg126003
2011-01-10 19:33:52eric.araujosetnosy: gvanrossum, rhettinger, pitrou, benjamin.peterson, eric.araujo, daniel.urban, kerio
messages: + msg125915
2011-01-10 19:25:13keriosetfiles: + abcmeta_register_v4_plus_tests.diff
nosy: gvanrossum, rhettinger, pitrou, benjamin.peterson, eric.araujo, daniel.urban, kerio
2011-01-10 19:23:42keriosetfiles: - abcmeta_register_v4_plus_tests.diff
nosy: gvanrossum, rhettinger, pitrou, benjamin.peterson, eric.araujo, daniel.urban, kerio
2011-01-10 18:58:00keriosetfiles: + abcmeta_register_v4_plus_tests.diff
nosy: gvanrossum, rhettinger, pitrou, benjamin.peterson, eric.araujo, daniel.urban, kerio
messages: + msg125912
2011-01-10 17:57:32eric.araujosetnosy: gvanrossum, rhettinger, pitrou, benjamin.peterson, eric.araujo, daniel.urban, kerio
messages: + msg125909
2011-01-10 17:51:53keriosetnosy: + rhettinger, eric.araujo, gvanrossum, benjamin.peterson, pitrou, daniel.urban
2011-01-10 17:51:16keriosetfiles: + abcmeta_register_v4.diff
nosy: - gvanrossum, rhettinger, pitrou, benjamin.peterson, eric.araujo, daniel.urban
messages: + msg125907

2011-01-10 17:36:08daniel.urbansetnosy: + daniel.urban
messages: + msg125905
2011-01-09 00:30:38rhettingersetnosy: + rhettinger
messages: + msg125816
2011-01-09 00:19:28eric.araujosetfiles: + abcmeta_register_v3.diff
nosy: gvanrossum, pitrou, benjamin.peterson, eric.araujo, kerio
messages: + msg125814
2011-01-09 00:05:42keriosetfiles: + abcmeta_register_deco_with_docs.diff
nosy: gvanrossum, pitrou, benjamin.peterson, eric.araujo, kerio
messages: + msg125810
2011-01-08 23:41:11eric.araujosetassignee: eric.araujo

messages: + msg125807
nosy: + eric.araujo
2011-01-08 23:33:14pitrousetnosy: + gvanrossum, pitrou, benjamin.peterson

messages: + msg125806
stage: patch review
2011-01-08 23:32:11keriocreate