confusion with decorators

J

Jason Swails

Hello,

I was having some trouble understanding decorators and inheritance and all
that. This is what I was trying to do:

# untested
class A(object):
def _protector_decorator(fcn):
def newfcn(self, *args, **kwargs):
return fcn(self, *args, **kwargs)
return newfcn

@_protector_decorator
def my_method(self, *args, **kwargs):
""" do something here """

class B(A):
def _protector_decorator(fcn):
def newfcn(self, *args, **kwargs):
raise MyException('I do not want B to be able to access the
protected functions')
return newfcn

The goal of all that was to be able to change the behavior of my_method
inside class B simply by redefining the decorator. Basically, what I want
is B.my_method() to be decorated by B._protector_decorator, but in the code
I'm running it's decorated by A._protector_decorator.

I presume this is because once the decorator is applied to my_method in
class A, A.my_method is immediately bound to the new, 'decorated' function,
which is subsequently inherited (and not decorated, obviously), by B.

Am I correct here? My workaround was to simply copy the method from class
A to class B, after which B._protector_decorator decorated the methods in
B. While this doesn't make the use of decorators completely pointless (the
decorators actually do something in each class, it's just different), it
does add a bunch of code duplication which I was at one point hopeful to
avoid.

I'm still stumbling around with decorators a little, but this exercise has
made them a lot clearer to me.

Thanks!
Jason
 
S

Steven D'Aprano

Hello,

I was having some trouble understanding decorators and inheritance and
all that. This is what I was trying to do:

# untested
class A(object):
def _protector_decorator(fcn):
def newfcn(self, *args, **kwargs):
return fcn(self, *args, **kwargs)
return newfcn

Well, that surely isn't going to work, because it always decorates the
same function, the global "fcn".

You probably want to add an extra parameter to the newfcn definition:

def newfcn(self, fcn, *args, **kwargs):


Also, I trust you realise that this is a pointless decorator that doesn't
do anything useful? It just adds an extra layer of indirection, without
adding any functionality.

@_protector_decorator
def my_method(self, *args, **kwargs):
""" do something here """

class B(A):
def _protector_decorator(fcn):
def newfcn(self, *args, **kwargs):
raise MyException('I do not want B to be able to access the
protected functions')
return newfcn


That's not going to work, because B's _protector_decorator never gets
called. True, it overrides A's _protector_decorator, but too late. A has
already used it to decorate the methods, and B does not override those
methods, so A's version are inherited.

But even if it could work, it relies on class B protecting class A from
B. All B needs do to overcome the protection is ... *not* define the
magic decorator.

The goal of all that was to be able to change the behavior of my_method
inside class B simply by redefining the decorator. Basically, what I
want is B.my_method() to be decorated by B._protector_decorator, but in
the code I'm running it's decorated by A._protector_decorator.

Yes. Remember that you don't have a B.my_method, so B merely inherits
A.my_method.

I presume this is because once the decorator is applied to my_method in
class A, A.my_method is immediately bound to the new, 'decorated'
function, which is subsequently inherited (and not decorated,
obviously), by B.
Correct.

Am I correct here? My workaround was to simply copy the method from
class A to class B, after which B._protector_decorator decorated the
methods in B.

That's not a work-around, that's an anti-pattern.

Why is B inheriting from A if you don't want it to be able to use A's
methods? That's completely crazy, if you don't mind me saying so. If you
don't want B to access A's methods, simply don't inherit from A.

I really don't understand what you are trying to accomplish here.
Possibly Java.

http://dirtsimple.org/2004/12/python-is-not-java.html
http://dirtsimple.org/2004/12/java-is-not-python-either.html


But you can accomplish something close to what you are after like this:


import functools

def decorate(func):
@functools.wraps(func)
def inner(self, *args, **kwargs):
protector = getattr(self, '_protect', None)
if protector is not None:
protector()
return func(self, *args, **kwargs)
return inner


class A(object):
@decorate
def mymethod(self):
"""Do something useful."""


class B(A):
def _protect(self):
raise RuntimeError("I'm sorry Dave, I'm afraid I cannot do that.")



Try studying that to see how it works, and then try studying it to
realise how pointless it is, since it too relies on class B protecting
class A from B.
 
J

Jason Swails

On Thu, Jan 31, 2013 at 12:46 AM, Steven D'Aprano <
Well, that surely isn't going to work, because it always decorates the
same function, the global "fcn".

I don't think this is right. fcn is a passed function (at least if it acts
as a decorator) that is declared locally in the _protector_decorator scope.
Since newfcn is bound in the same scope and fcn is not defined inside
newfcn, I'm pretty sure that newfcn will just grab the fcn passed into the
decorator.

The following code illustrates what I'm trying to say (I think):

test.py:
#!/usr/bin/env python

a = 3

print 'Global namespace:', a

def myfunc(a):
def nested_func():
print 'nested_func a is:', a, 'id(a) =', id(a)

print 'passed a is:', a, 'id(a) = ', id(a)
nested_func()

myfunc(10)

$ python test.py
Global namespace: 3
passed a is: 10 id(a) = 6416096
nested_func a is: 10 id(a) = 6416096

Likewise, newfcn will use the function bound to the passed argument to the
decorator. This syntax appears to work in my 'real' program.

You probably want to add an extra parameter to the newfcn definition:

def newfcn(self, fcn, *args, **kwargs):

I don't think I want to do that, since fcn will simply become the first
argument that I pass to the decorated myfunc(), and if it's not callable
I'll get a traceback.

Also, I trust you realise that this is a pointless decorator that doesn't
do anything useful? It just adds an extra layer of indirection, without
adding any functionality.

Naturally. I tried to contrive the simplest example to demonstrate what I
wanted. In retrospect I should've picked something functional instead.
Am I correct here? My workaround was to simply copy the method from

That's not a work-around, that's an anti-pattern.

Why is B inheriting from A if you don't want it to be able to use A's
methods? That's completely crazy, if you don't mind me saying so. If you
don't want B to access A's methods, simply don't inherit from A.

I really don't understand what you are trying to accomplish here.

Again, my example code is over-simplified. A brief description of my class
is a list of 'patch' (diff) files with various attributes. If I want
information from any of those files, I instantiate a Patch instance (and
cache it for later use if desired) and return any of the information I want
from that patch (like when it was created, who created it, what files will
be altered in the patch, etc.).

But a lot of these patches are stored online, so I wanted a new class (a
RemotePatchList) to handle lists of patches in an online repository. I can
do many of the things with an online patch that I can with one stored
locally, but not everything, hence my desire to squash the methods I don't
want to support.

I'd imagine a much more sensible approach is to generate a base class that
implements all methods common to both and simply raises an exception in
those methods that aren't. I agree it doesn't make much sense to inherit
from an object that has MORE functionality than you want.

However, my desire to use decorators was not to disable methods in one
class vs. another. The _protector_decorator (a name borrowed from my
actual code), is designed to wrap a function call inside a try/except, to
account for specific exceptions I might raise inside. One of my classes
deals with local file objects, and the other deals with remote file objects
via urllib. Naturally, the latter has other exceptions that can be raised,
like HTTPError and the like. So my desire was to override the decorator to
handle more types of exceptions, but leave the underlying methods intact
without duplicating them.

I can do this without decorators easily enough, but I thought the decorator
syntax was a bit more elegant and I saw an opportunity to learn more about
them.

Possibly Java.
I took a Java class in high school once ~10 years ago... haven't used it
since. :) Truth be told, outside of Python, the languages I can work in
are Fortran (and to a much lesser extent), C and C++.

import functools
I need to support Python 2.4, and the docs suggest this is 2.5+. Too bad,
too, since functools appears pretty useful.

Thanks for the help!
Jason
 
C

Chris Angelico

I don't think this is right. fcn is a passed function (at least if it acts
as a decorator) that is declared locally in the _protector_decorator scope.
Since newfcn is bound in the same scope and fcn is not defined inside
newfcn, I'm pretty sure that newfcn will just grab the fcn passed into the
decorator.

Yet it adds a level of indirection that achieves nothing. Why not simply:
def _protector_decorator(fcn):
return fcn

? I'm not understanding the purpose here.

ChrisA
 
J

Jason Swails

Yet it adds a level of indirection that achieves nothing. Why not simply:
def _protector_decorator(fcn):
return fcn

? I'm not understanding the purpose here.

Bad example. A better (longer) one that is closer to my true use-case:


from somewhere.exceptions import MyTypeError
from somewhere.different import AuthorClass, RemoteAuthorClass
from urllib2 import HTTPError

class A(object):

authorclass = AuthorClass

def __init__(self, obj_list):
"""
Instantiate a list of obj_list objects that may have an "author"
attribute
"""
self.things = []
for o in obj_list:
if not isinstance(o, self.authorclass):
raise MyTypeError('Bad type given to constructor')
self.things.append(o)

def _protector(fcn):
def newfcn(self, *args, **kwargs):
try:
return fcn(self, *args, **kwargs) # returns a string
except AttributeError:
return 'Attribute not available.'
except IndexError:
return 'Not that many AuthorClasses loaded'

return newfcn

@_protector
def author(self, idx):
return self.things[idx].author

@_protector
def description(self, idx):
return self.things[idx].description

@_protector
def hobbies(self, idx):
return self.things[idx].hobbies

class B(A):

authorclass = RemoteAuthorClass

def _protector(fcn):
def newfcn(self, *args, **kwargs):
try:
return fcn(self, *args, **kwargs)
except AttributeError:
return 'Attribute not available'
except IndexError:
return 'Not that many RemoteAuthorClasses loaded'
except HTTPError:
return 'Could not connect'
return fcn

Basically, while RemoteAuthorClass and AuthorClass are related (via
inheritance), the RemoteAuthorClass has the potential for HTTPError's now.
I could just expand the A class decorator to catch the HTTPError, but
since that should not be possible in AuthorClass, I'd rather not risk
masking a bug. I'm under no impressions that the above code will decorate
A-inherited functions with the B-decorator (I know it won't), but that's
the effect I'm trying to achieve...

Thanks!
Jason

--
Jason M. Swails
Quantum Theory Project,
University of Florida
Ph.D. Candidate
352-392-4032
 
J

Jason Swails

Yet it adds a level of indirection that achieves nothing. Why not simply:
def _protector_decorator(fcn):
return fcn

? I'm not understanding the purpose here.

Bad example. A better (longer) one that is closer to my true use-case:


from somewhere.exceptions import MyTypeError
from somewhere.different import AuthorClass, RemoteAuthorClass
from urllib2 import HTTPError

class A(object):

authorclass = AuthorClass

def __init__(self, obj_list):
"""
Instantiate a list of obj_list objects that may have an "author"
attribute
"""
self.things = []
for o in obj_list:
if not isinstance(o, self.authorclass):
raise MyTypeError('Bad type given to constructor')
self.things.append(o)

def _protector(fcn):
def newfcn(self, *args, **kwargs):
try:
return fcn(self, *args, **kwargs) # returns a string
except AttributeError:
return 'Attribute not available.'
except IndexError:
return 'Not that many AuthorClasses loaded'

return newfcn

@_protector
def author(self, idx):
return self.things[idx].author

@_protector
def description(self, idx):
return self.things[idx].description

@_protector
def hobbies(self, idx):
return self.things[idx].hobbies

class B(A):

authorclass = RemoteAuthorClass

def _protector(fcn):
def newfcn(self, *args, **kwargs):
try:
return fcn(self, *args, **kwargs)
except AttributeError:
return 'Attribute not available'
except IndexError:
return 'Not that many RemoteAuthorClasses loaded'
except HTTPError:
return 'Could not connect'
return fcn

Basically, while RemoteAuthorClass and AuthorClass are related (via
inheritance), the RemoteAuthorClass has the potential for HTTPError's now.
I could just expand the A class decorator to catch the HTTPError, but
since that should not be possible in AuthorClass, I'd rather not risk
masking a bug. I'm under no impressions that the above code will decorate
A-inherited functions with the B-decorator (I know it won't), but that's
the effect I'm trying to achieve...

The approach I'm switching to here is to make the decorators wrappers
instead that are passed the functions that need to be called. Basically,
wrap at run-time rather than 'compile time' (i.e., when the Python code is
'compiled' into class definitions). That way each child of the main class
can simply change the wrapping behavior by implementing the wrapping
functions instead of duplicating all of the code. And since this part of
the code is not performance-intensive, I don't care about the overhead of
extra function calls.

It seems to me to be the more appropriate course of action here, since
decorators don't seem to naturally lend themselves to what I'm trying to do.

--Jason
 
S

Steven D'Aprano

Steven said:
Well, that surely isn't going to work, because it always decorates the
same function, the global "fcn".

Good grief, I can't believe I failed to see that fcn was declared as a
parameter to _protector_decorator.

You probably want to add an extra parameter to the newfcn definition:

def newfcn(self, fcn, *args, **kwargs):

And that's also rubbish. The right place for the fcn parameter is the
decorator function itself, exactly where it already is.

Whatever crack I was smoking yesterday, it must have been pretty awful
stuff.
 
S

Steven D'Aprano

Jason said:
I don't think this is right.

It certainly isn't. Sorry for the noise.


[...]
Again, my example code is over-simplified. A brief description of my
class
is a list of 'patch' (diff) files with various attributes. If I want
information from any of those files, I instantiate a Patch instance (and
cache it for later use if desired) and return any of the information I
want from that patch (like when it was created, who created it, what files
will be altered in the patch, etc.).

But a lot of these patches are stored online, so I wanted a new class (a
RemotePatchList) to handle lists of patches in an online repository. I
can do many of the things with an online patch that I can with one stored
locally, but not everything, hence my desire to squash the methods I don't
want to support.


Normally, subclasses should extend functionality, not take it away. A
fundamental principle of OO design is that anywhere you could sensibly
allow an instance, should also be able to use a subclass.

So if you have a Patch class, and a RemotePatch subclass, then everything
that a Patch can do, a RemotePatch can do too, because RemotePatch
instances *are also* instances of Patch.

But the rule doesn't go in reverse: you can't necessarily use a Patch
instance where you were using a RemotePatch. Subclasses are allowed to do
*more*, but they shouldn't do *less*.

On the other hand, if you have a Patch class, and a RemotePatchList class,
inheritance does not seem to be the right relationship here. A
RemotePatchList does not seem to be a kind of Patch, but a kind of list.

I'd imagine a much more sensible approach is to generate a base class that
implements all methods common to both and simply raises an exception in
those methods that aren't. I agree it doesn't make much sense to inherit
from an object that has MORE functionality than you want.

If a method is not common to both, it doesn't belong in the base class. The
base should only include common methods.

In fact, I'm usually rather suspicious of base classes that don't ever get
used except as a base for subclassing. I'm not saying it's wrong, but it
could be excessive abstraction. Abstraction is good, but you can have too
much of a good thing. If the base class is not used, consider a flatter
hierarchy:

class Patch: ...
class RemotePatch(Patch): ...


rather than:

class PatchBase: ...
class Patch(PatchBase): ...
class RemotePatch(Patch): ...

although this is okay:

class PatchBase: ...
class Patch(PatchBase): ...
class RemotePatch(PatchBase): ...

However, my desire to use decorators was not to disable methods in one
class vs. another. The _protector_decorator (a name borrowed from my
actual code), is designed to wrap a function call inside a try/except, to
account for specific exceptions I might raise inside.

Ah, your example looked like you were trying to implement some sort of
access control, where some methods were flagged as "protected" to prevent
subclasses from using them. Hence my quip about Java. What you describe
here makes more sense.

One of my classes
deals with local file objects, and the other deals with remote file
objects
via urllib. Naturally, the latter has other exceptions that can be
raised,
like HTTPError and the like. So my desire was to override the decorator
to handle more types of exceptions, but leave the underlying methods
intact without duplicating them.
4

One way to do that is to keep a list of exceptions to catch:


class Patch:
catch_these = [SpamException, HamException]
def method(self, arg):
try:
do_this()
except self.catch_these:
do_that()

The subclass can then extend or replace that list:

class RemotePatch(Patch):
catch_these = Patch.catch_these + [EggsException, CheeseException]



I need to support Python 2.4, and the docs suggest this is 2.5+. Too bad,
too, since functools appears pretty useful.

functools.wraps is pretty simple. You can use this as an equivalent:

# `functools.wraps` was added in Python 2.5.
def wraps(func_to_wrap):
"""Return a decorator that wraps its argument.

This is a reimplementation of functools.wraps() which copies the name,
module, docstring and attributes of the base function to the decorated
function. wraps() is available in the standard library from Python 2.5.
... '''This is a doc string.'''
... return x+1
...... def decorated(x):
... return undecorated(x)
...'undecorated'

"""
def decorator(func):
def f(*args, **kwargs):
return func(*args, **kwargs)
f.__doc__ = func_to_wrap.__doc__
try:
f.__name__ = func_to_wrap.__name__
except Exception:
# Older versions of Python (2.3 and older perhaps?)
# don't allow assigning to function __name__.
pass
f.__module__ = func_to_wrap.__module__
if hasattr(func_to_wrap, '__dict__'):
f.__dict__.update(func_to_wrap.__dict__)
return f
return decorator


The doctest passes for Python 2.4.
 
J

Jason Swails

On Thu, Jan 31, 2013 at 6:16 PM, Steven D'Aprano <
Normally, subclasses should extend functionality, not take it away. A
fundamental principle of OO design is that anywhere you could sensibly
allow an instance, should also be able to use a subclass.

So if you have a Patch class, and a RemotePatch subclass, then everything
that a Patch can do, a RemotePatch can do too, because RemotePatch
instances *are also* instances of Patch.

But the rule doesn't go in reverse: you can't necessarily use a Patch
instance where you were using a RemotePatch. Subclasses are allowed to do
*more*, but they shouldn't do *less*.

On the other hand, if you have a Patch class, and a RemotePatchList class,
inheritance does not seem to be the right relationship here. A
RemotePatchList does not seem to be a kind of Patch, but a kind of list.



If a method is not common to both, it doesn't belong in the base class. The
base should only include common methods.

Yes, I agree here. The only reason I was considering NOT doing this was
because I wanted to control the exception that gets raised rather than let
through a simple NameError. The reason, in case you care, is that I like
creating my own custom excepthook() which optionally suppresses tracebacks
of the base exception class of my program (which can be overridden by a
--debug option of some sort).

That way I don't worry about returning error codes and the like and my
exceptions double as error messages which don't scare users away. Of
course, if I didn't raise the exception myself, then I definitely want to
know what line that error occurred on so I can fix it (since that typically
means it's a bug or error I did not handle gracefully).

I suppose I could get the same effect by dumping everything into a main()
function somewhere and wrapping that in a try/except where I catch my base
class, but I like the flexibility

In fact, I'm usually rather suspicious of base classes that don't ever get
used except as a base for subclassing. I'm not saying it's wrong, but it
could be excessive abstraction. Abstraction is good, but you can have too
much of a good thing. If the base class is not used, consider a flatter
hierarchy:

class Patch: ...
class RemotePatch(Patch): ...


rather than:

class PatchBase: ...
class Patch(PatchBase): ...
class RemotePatch(Patch): ...

although this is okay:

class PatchBase: ...
class Patch(PatchBase): ...
class RemotePatch(PatchBase): ...

This last one is what I've settled on. Patch and RemotePatch have common
functionality. But RemotePatch can be downloaded and Patch can be parsed
through (in my app, if you're going to spend the time to parse through the
whole RemotePatch, it just gets downloaded and instantiated as a Patch).
So this last form of inheritance made the most sense to me.

However, my desire to use decorators was not to disable methods in one
class vs. another. The _protector_decorator (a name borrowed from my
actual code), is designed to wrap a function call inside a try/except, to
account for specific exceptions I might raise inside.

Ah, your example looked like you were trying to implement some sort of
access control, where some methods were flagged as "protected" to prevent
subclasses from using them. Hence my quip about Java. What you describe
here makes more sense.

One of my classes
deals with local file objects, and the other deals with remote file
objects
via urllib. Naturally, the latter has other exceptions that can be
raised,
like HTTPError and the like. So my desire was to override the decorator
to handle more types of exceptions, but leave the underlying methods
intact without duplicating them.
4

One way to do that is to keep a list of exceptions to catch:


class Patch:
catch_these = [SpamException, HamException]
def method(self, arg):
try:
do_this()
except self.catch_these:
do_that()

The subclass can then extend or replace that list:

class RemotePatch(Patch):
catch_these = Patch.catch_these + [EggsException, CheeseException]

Ha! I use this technique all the time to avoid code duplication (it's used
several times in the program I'm writing). It didn't even occur to me in
this context... Thanks for pointing this out!

As always, the time you put into responses and helping is appreciated.

All the best,
Jason
 
8

88888 Dihedral

Jason Swailsæ–¼ 2013å¹´1月31日星期四UTC+8上åˆ8時34分03秒寫é“:
Hello,


I was having some trouble understanding decorators and inheritance and all that.  This is what I was trying to do:



# untested
class A(object):
   def _protector_decorator(fcn):

      def newfcn(self, *args, **kwargs):
         return fcn(self, *args, **kwargs)
      return newfcn



   @_protector_decorator
   def my_method(self, *args, **kwargs):
      """ do something here """



class B(A):
   def _protector_decorator(fcn):
      def newfcn(self, *args, **kwargs):

         raise MyException('I do not want B to be able to access the protected functions')
      return newfcn



The goal of all that was to be able to change the behavior of my_method inside class B simply by redefining the decorator. Basically, what I want isB.my_method() to be decorated by B._protector_decorator, but in the code I'm running it's decorated by A._protector_decorator.



I presume this is because once the decorator is applied to my_method in class A, A.my_method is immediately bound to the new, 'decorated' function, which is subsequently inherited (and not decorated, obviously), by B.



Am I correct here?  My workaround was to simply copy the method fromclass A to class B, after which B._protector_decorator decorated the methods in B.  While this doesn't make the use of decorators completely pointless (the decorators actually do something in each class, it's just different), it does add a bunch of code duplication which I was at one point hopeful to avoid.



I'm still stumbling around with decorators a little, but this exercise has made them a lot clearer to me.


Thanks!
Jason

It sounds that you need a decorator mapper to
perform the functionality of your designs.
 
8

88888 Dihedral

Jason Swailsæ–¼ 2013å¹´1月31日星期四UTC+8上åˆ8時34分03秒寫é“:
Hello,


I was having some trouble understanding decorators and inheritance and all that.  This is what I was trying to do:



# untested
class A(object):
   def _protector_decorator(fcn):

      def newfcn(self, *args, **kwargs):
         return fcn(self, *args, **kwargs)
      return newfcn



   @_protector_decorator
   def my_method(self, *args, **kwargs):
      """ do something here """



class B(A):
   def _protector_decorator(fcn):
      def newfcn(self, *args, **kwargs):

         raise MyException('I do not want B to be able to access the protected functions')
      return newfcn



The goal of all that was to be able to change the behavior of my_method inside class B simply by redefining the decorator. Basically, what I want isB.my_method() to be decorated by B._protector_decorator, but in the code I'm running it's decorated by A._protector_decorator.



I presume this is because once the decorator is applied to my_method in class A, A.my_method is immediately bound to the new, 'decorated' function, which is subsequently inherited (and not decorated, obviously), by B.



Am I correct here?  My workaround was to simply copy the method fromclass A to class B, after which B._protector_decorator decorated the methods in B.  While this doesn't make the use of decorators completely pointless (the decorators actually do something in each class, it's just different), it does add a bunch of code duplication which I was at one point hopeful to avoid.



I'm still stumbling around with decorators a little, but this exercise has made them a lot clearer to me.


Thanks!
Jason

It sounds that you need a decorator mapper to
perform the functionality of your designs.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

No members online now.

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,904
Latest member
HealthyVisionsCBDPrice

Latest Threads

Top