[Urwid] possible bug: AttrMap does not pass function calls/property lookups to the wrapped widget

[Vlad]

Greetings,

I've built an urwid application that makes use of several urwid features: it looks a bit like a spreadsheet with editable/tab-able cells and expandable rows, uses Columns, Buttons, Frames, as well as widgets that are extensions of urwid widgets or WidgetWrap. I can say at this point that urwid is a very useful framework and fits its niche very well.

Over the course of building this app I noticed something that might be a bug or ?user error?:

- contrary to the documentation, AttrMap does not appear to forward method calls to ?w?, the wrapped widget. Here is a quick repro, a modification of Hello World:

import urwid

palette = [
('body', 'white', 'black'),
]

txt = urwid.Text(u"Hello World")
txt = urwid.AttrMap (txt, 'body')
#txt.original_widget.set_text (u"Goodbye World") # this works
txt.set_text (u"Goodbye World") # this doesn?t (but will if you use the deprecated AttrWrap)
fill = urwid.Filler(txt, 'top')
loop = urwid.MainLoop(fill)
loop.run()

I find AttrMap to be a very useful design concept. But I think it falls short of its intended purpose because I find myself constantly needing to insert .base_widget in various places ? AttrMap is not as transparent as the docs suggest. A piece of urwid code will work but later break when you add an extra AttrMap layer somewhere. Note that using AttrWrap instead makes the example above work ? even though AttrWrap is deprecated in favor of AttrMap. Several stock urwid examples also break if AttrWrap is replaced with AttrMap, and it originally took me a few hours to figure out why.

HTH,
Vlad


-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.excess.org/pipermail/urwid/attachments/20140217/a3574019/attachment.htm

[Ian Ward]

Post by Vlad
Over the course of building this app I noticed something that might be a bug
- contrary to the documentation, AttrMap does not appear to forward method
calls to 'w', the wrapped widget.

Oh, you're absolutely right. There is a bug in the documentation.

I decided that it was a mistake for AttrWrap to pass all attribute
access and method calls to the wrapped class and I deliberately did
not include that feature in AttrMap. It seems I forgot to update the
docstring.

There are many type of decoration widgets in Urwid. If we pass through
attribute access for just one decoration, why not do it for all
decorations? If we do it for all decorations then users will be able
to mostly pretend the decorations aren't there, but that could lead to
some really hard to understand code. It mixes the namespace of every
decoration with the base widget's namespace.

Post by Vlad
I find AttrMap to be a very useful design concept. But I think it falls
short of its intended purpose because I find myself constantly needing to
insert .base_widget in various places -- AttrMap is not as transparent as the
docs suggest. A piece of urwid code will work but later break when you add
an extra AttrMap layer somewhere. Note that using AttrWrap instead makes the
example above work -- even though AttrWrap is deprecated in favor of AttrMap.
Several stock urwid examples also break if AttrWrap is replaced with
AttrMap, and it originally took me a few hours to figure out why.

I agree that it should be easier to work with decoration widgets. The
best idea I have at the moment is to add a 'decorations' property or
'get_decorations()' method to widgets that will be called whenever
that widget is added to a container or assigned to loop.widget. This
way a widget class can define all its own decorations but still
present just its methods and attributes to the programmer.

How does that approach sound to you?

Ian

[Vlad]

Post by Ian Ward

Post by Vlad
I find AttrMap to be a very useful design concept. But I think it falls
short of its intended purpose because I find myself constantly needing to
insert .base_widget in various places -- AttrMap is not as transparent as the
docs suggest. A piece of urwid code will work but later break when you add
an extra AttrMap layer somewhere. Note that using AttrWrap instead makes the
example above work -- even though AttrWrap is deprecated in favor of AttrMap.
Several stock urwid examples also break if AttrWrap is replaced with
AttrMap, and it originally took me a few hours to figure out why.

I agree that it should be easier to work with decoration widgets. The
best idea I have at the moment is to add a 'decorations' property or
'get_decorations()' method to widgets that will be called whenever
that widget is added to a container or assigned to loop.widget. This
way a widget class can define all its own decorations but still
present just its methods and attributes to the programmer.
How does that approach sound to you?

I think it is necessary to make a design choice: are decorations Widgets in their own right or not (i.e. some properties of Widgets instead). The frame of mind I had been in when developing my app (which is where I think the docs lead you) was something like ?AttrMap is a Widget that extends the Widget that it wraps, but it ?overrides? that Widget?s attribute mapping). An "abbreviated" Composite pattern where AttrMap is a Widget and contains (just one, hence ?abbreviated") another Widget. From this point of view, AttrMappingWidget or DecoratedWidget may be better names and it feels natural for it to ?extend" the widget that it also wraps ? then passing all methods and properties through makes sense and ?.base_widget? is like ?.super?.

An alternative design could be: don?t attempt to use a Composite pattern for decorations at all, because it would be a parallel Composite to the one that many are already used to: container Widgets. I.e. Columns is WidgetContainerMixin and it totally makes sense given how events like key strokes and mouse clicks are propagated. So, in this design choice we have a Decoration that is not a Widget and a Widget can contain multiple Decorations. But that raises more questions: if a Widget aggregates one or many Decorations, in which order are they applied? I am pretty sure that all possible answers here will only lead to a more confusing framework.

So, FWIW, I think it is best for a Decoration to accept a Widget as a constructor argument (i.e. to ?wrap? it) but at the same time to be a Widget itself (which implies forwarding all methods/properties to the Widget being decorated by default). I.e. make the former choice above. I am not sure if this is the most pythonic choice, but it has been done with success before, see http://www.onjava.com/pub/a/onjava/2003/02/05/decorator.html The end users have two choices for extending and tweaking Widgets: by subclassing them or by wrapping them into Decorations (or Decorators). The former is a choice that affects all instances of the derived class and the latter decorates a single instance, so they are complementary.

As you can guess by now, I prefer the first choice. When you ask "If we pass through attribute access for just one decoration, why not do it for all decorations? If we do it for all decorations then users will be able to mostly pretend the decorations aren't there, but that could lead to some really hard to understand code? I answer ?Yes? and ?It?s a good thing that decorations can be transparent ? it is compatible with them being Widgets?. When I write ChangeBackgroundColorDecorator(w) I find it natural not to know whether ?w? is a ?primal? Widget or has already been decorated ? it shouldn?t matter. The order of wrapping defines the order of decoration, so that?s all very natural, too. And this ?pretending that the decorations aren?t there? is actually a good thing, as it will cause less coupling between various classes and more concern separation. Right now, every ?.base_widget? feels like a speedbump...

HTH,
Vlad