> -----Original Message----- > From: Darren Dale [mailto:dsdal...@gmail.com] > > On Thu, Aug 13, 2009 at 7:44 AM, John Hunter<jdh2...@gmail.com> wrote: > > On Wed, Aug 12, 2009 at 7:12 AM, John Hunter<jdh2...@gmail.com> > wrote: > I appreciate how much time has gone into the patch, but this impacts > so much code I think it is important to really nail the > implementation.
Agreed
> I think everything should be rolled up into a single
> high level decorator if possible.
While I think this approach is useful for readability where a pattern is seen
repeated, trying to implement every matplotlib docstring manipulation into a
single, monolithic decorator has the potential to become messy. This is why
I've chosen to implement the same functionality in various components that can
(and should in some cases) be assembled into a single construct.
> I offered a suggestion in the
> tracker. Consider:
>
> with_doc(*args, **kwargs):
> mod_doc(f):
> if args:
> if not isinstance(args[0], str):
> # inherit docstring from first argument
> inherit = args.pop(0).__doc__
> if f.__doc__ is None:
> f.__doc__ = inherit
> else:
> f.__doc__ = inherit + '\n\n' + f.__doc__
> if kwargs:
> # mapping interpolation
> f.__doc__ = f.__doc__ % kwargs
> if args:
> try:
> # inserting
> f.__doc__ = f.__doc__ % args
> except TypeError:
> # appending
> if f.__doc__ is None:
> f.__doc__ = ''
> f.__doc__ = f.__doc__ + '\n\n'+ '\n\n'.join(args)
> return f
> return mod_doc
I appreciate the suggested implementation. It helped inspire me with my
submitted patch, which I believe meets the same goals (or can with some
combination of some of the various decorators).
This suggested implementation however has a few drawbacks compared with the
submitted patch.
1) The form doesn't reflect the function. mod_doc indicates that the
documentation is being modified, but the function is implicit and has to be
reverse-engineered from the code. On the other hand, docstring.copy copies a
docstring, while docstring.Substitution performs substitution.
2) with_doc doesn't have a way to handle a mutable parameter set. That is,
all of the **kwargs or *args (in the second usage) must be present at the time
the decorator is created. In the case of the sub_args decorator in artist.py,
this isn't suitable. It would be possible to just create a new decorator
(i.e. with_doc(**artist.kwdocd)) every place of sub_args is used, but that
violates the DRY principle. The sub_args mutable decorator, on the other hand,
takes the place of artist.kwdocd by representing an object with purpose and
not just kwdocd data structure.
3) with_doc allows for both keyword and positional argument substitution.
This seems like a dangerous allowance.
4) Modular decorators can have delegated responsibility and be tested
separately.
5) with_doc hard-codes separators ('\n\n') in multiple places and doesn't
allow a different separator to be specified (if necessary).
6) The TypeError exception handler in the second usage of *args may trap more
exceptions than desired.
7) As you mentioned, it doesn't handle dedent. It's not clear to me where
dedent should be included in the logic to properly capture the needs of
matplotlib.
8) with_doc is largely addressing issues specifically around matplotlib. The
submitted patch provides reusable decorators which might be assembled in
creative ways not yet conceived.
Please understand, I'm trying to explain why the current implementation is
sound and not to criticize your contribution.
> I left out the dedentation for now, and it is untested. This can cover
> lots of ground, I think it can be chained on itself:
>
> @with_doc('append me', prepend='prepend me as well')
> @with_doc(other_method)
> @with_doc('prepend me %(prepend)s')
The current code allows
@docstring.Substitution(prepend='prepend me as well')
@docstring.Appender('append me', join='\n\n')
@docstring.copy(other_method)
@docstring.Appender('prepend me %(prepend)s', join='\n\n')
While slightly more verbose, the second technique communicates clearly what is
happening and there's no implicit behavior going on (i.e. copy takes another
method while Appender takes a string). The default join could be '\n\n', but
I chose an empty string as default. If '\n\n' is more desirable, that could
certainly be made the default.
> If it is not possible, or possible but messy because of dedentation or
> other reasons, to pack everything into a single high-level decorator,
> then maybe there should be a few like:
>
> inherit_doc(fun) overwrite or prepend with fun's docstring
This is essentially the purpose of docstring.copy.
> insert_doc(*args, **kwargs) interpolate
This is the behavior of docstring.Substitution (plus allows for mutable
parameters).
> append_doc(*args)
This is docstring.Appender (except only allows one argument, and allows
separator to be supplied). If multiple args are require for appending, that
certainly could be implemented.
> but since these are high-level decorators, chaining decorators would
> result in repeated calls to dedent.
>From that, high-level decorators could be assembled with the appropriate
dedent behavior included as well.
smime.p7s
Description: S/MIME cryptographic signature
------------------------------------------------------------------------------ Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day trial. Simplify your report design, integration and deployment - and focus on what you do best, core application coding. Discover what's new with Crystal Reports now. http://p.sf.net/sfu/bobj-july
_______________________________________________ Matplotlib-devel mailing list Matplotlib-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
