matplotlib-devel Mailing List for matplotlib

John Hunter wrote:
>>>>>>"Baptiste" == Baptiste Carvello <bap...@al...> writes:
> 
> 
> Baptiste> I know the patch is intrusive, though maybe not as much
> Baptiste> as it looks (everything except protect_mutex.py is
> Baptiste> pretty trivial). I didn't expected it to be accepted
> Baptiste> right away, I see it more as a contribution to the whole
> Baptiste> gui thread discution.
> 
> Baptiste> As for gui_thread, I looked at it last year, I just
> Baptiste> remember I didn't like that code that much. Maybe I
> Baptiste> should get another look...
> 
> Most of what I've heard of gui thread (caveat, the situation may be
> different now but this was the last I heard) was that the code was
> hairy, required scipy distutils to build, was wx specific and had
> problems that required significant work to fix. In that context, a
> patch, even an intrusive one, that allowed gtk* and wx* to work from
> an arbitrary python shell would be most welcome. While I think
> ipython is the one true shell, there are many other shells in
> existence and that won't change. If for example, this patch allowed
> gtk* to be used from within IDLE or pycrust in addition to the
> standard python shell, that would be a big win.
I certainly have never been a fan of digging too deep into gui_thread myself, 
I admit that much :) Last time I looked at the code, I got a bit scared. On 
the other hand, Prabhu recently did a bunch of work on it, so it may be 
cleaner/better, I just don't know.
And as I said, I really was not speaking from the ipython side of things at 
all, I don't know why Baptiste took it that way (I didn't even mention ipython 
in my post). My opinion was strictly based on the generic approach offered by 
gui_thread, and the potential benefit of doing things that way.
> The problem we need to solve is substantially easier than the one
> gui_thread wants to solve, if I understand the issues correctly. gui
> thread enables you to interact with general wx apps from a general
> python shell. We want gtk* and wx* backends to work from a general
> python shell. If this patch addresses that, it's worth a close look.
One thing that I like about gui_thread is that it works in a generic manner. 
What happens with this patch if someone uses matplotlib but adds new widgets? 
Or if they embed a matplotlib window into something else? Are such usage cases 
covered automatically, or do they require special handling by third-party code?
There certainly are benefits to a matplotlib-based self-contained solution, 
and gui_thread comes with its own baggage, so I'll let you guys judge where 
the balance rests.
Cheers,
f

>>>>> "Fernando" == Fernando Perez <Fer...@co...> writes:
 Fernando> Just a minor comment: insert(0) is an expensive (O(N))
 Fernando> operation on lists. This kind of problem might be better
 Fernando> addressed with a deque
 Fernando> (http://www.python.org/dev/doc/devel/lib/module-collections.html,
 Fernando> could be backported to py2.3).
If I read the patch correctly, usage_stack is being used as a simple
counter. 
I think the data structure we want is a class derived from dict that
pops elements off (preferably intelligently, ie pops the ones which
haven't been accessed in a while) when the dict length gets too long. 
In other words, we don't need to structures (cached and usage_stack)
to do the work of one
There must be such a beast in the cookbook or elsewhere, a cache
object which works like a dict but has a max size...
JDH 

>>>>> "Baptiste" == Baptiste Carvello <bap...@al...> writes:
 Baptiste> I know the patch is intrusive, though maybe not as much
 Baptiste> as it looks (everything except protect_mutex.py is
 Baptiste> pretty trivial). I didn't expected it to be accepted
 Baptiste> right away, I see it more as a contribution to the whole
 Baptiste> gui thread discution.
 Baptiste> As for gui_thread, I looked at it last year, I just
 Baptiste> remember I didn't like that code that much. Maybe I
 Baptiste> should get another look...
Most of what I've heard of gui thread (caveat, the situation may be
different now but this was the last I heard) was that the code was
hairy, required scipy distutils to build, was wx specific and had
problems that required significant work to fix. In that context, a
patch, even an intrusive one, that allowed gtk* and wx* to work from
an arbitrary python shell would be most welcome. While I think
ipython is the one true shell, there are many other shells in
existence and that won't change. If for example, this patch allowed
gtk* to be used from within IDLE or pycrust in addition to the
standard python shell, that would be a big win.
The problem we need to solve is substantially easier than the one
gui_thread wants to solve, if I understand the issues correctly. gui
thread enables you to interact with general wx apps from a general
python shell. We want gtk* and wx* backends to work from a general
python shell. If this patch addresses that, it's worth a close look.
Baptiste, have you have a chance to test with other shells? Would
your patch break gtk* or wx* under ipython? Does it work in IDLE
and/or pycrust? Do you have any ideas on how this would affect
performance? Obviously it adds some function call overhead -- with
collections the cases where there are large numbers of calls to the
backend renderers is reduced.
Another thought spoken as a threading newbie: could this be done more
simply by, eg subclassing RendererGTK and automating the protect
wrapping. This latter approach would satisfy a couple of points: it
would not be intrusive in the base class, could be made optional via
interactive setting or rc, and would not adversely impact performance
when it was not needed (on my quick read of the patch, it looks like
the last point is already handled by the call)
 if is_interactive():
 import_protect()
I'll give your patches a test-drive. Perhaps Steve could also comment
on the gtk changes.
JDH

Baptiste Carvello wrote:
> Hello,
> 
> I'm just making a proposal. As long as the documentation points to 
> IPython for interactive use, you can't say we have a perfect solution on 
> our own :-) (this is not meant to criticize your work on IPython, it's 
> just that I feel more confortable with the usual python interface)
> 
> I know the patch is intrusive, though maybe not as much as it looks 
> (everything except protect_mutex.py is pretty trivial). I didn't 
> expected it to be accepted right away, I see it more as a contribution 
> to the whole gui thread discution.
> 
> As for gui_thread, I looked at it last year, I just remember I didn't 
> like that code that much. Maybe I should get another look...
I actually didn't have ipython in mind at all in my comment. I agree that a 
generic, shell-independent solution is a good thing to have. I just think 
that gui_thread provides a cleaner approach (if not clean code :) to the 
overall problem. gui_thread is messy, but it allows general (meaning, beyond 
matplotlib) access to gui elements from a command line. I quickly tested it 
with matplotlib and it crashed, so there's work to be done...
I was just trying to point out an alternate direction, ultimately if John is 
happy with this approach, it's his call (he gets to maintain it :)
Cheers,
f

Hello,
I'm just making a proposal. As long as the documentation points to=20
IPython for interactive use, you can't say we have a perfect solution on=20
our own :-) (this is not meant to criticize your work on IPython, it's=20
just that I feel more confortable with the usual python interface)
I know the patch is intrusive, though maybe not as much as it looks=20
(everything except protect_mutex.py is pretty trivial). I didn't=20
expected it to be accepted right away, I see it more as a contribution=20
to the whole gui thread discution.
As for gui_thread, I looked at it last year, I just remember I didn't=20
like that code that much. Maybe I should get another look...
Cheers,
BC
Fernando Perez a =E9crit :
> Just a note: are you aware of the gui_thread module in scipy? It was=20
> written exactly to do this (though it's currently WX-specific, I think)=
,=20
> and it has seen a pretty extensive amount of work. I would much rather=
=20
> see a generic, external solution be tested and improved than a=20
> matplotlib-specific rather intrusive patch. Don't take it badly, I can=
=20
> tell you've put a lot of work into this, but this kind of problem is=20
> IMHO best addressed by a more generic mechanism. The kind of attribute=
=20
> access which python allows opens the door for such a solution, as=20
> gui_thread has shown.
>=20
> gui_thread currently needs testing and improvements, but I think in the=
=20
> long run it is a cleaner, better alternative.
>=20
> Just my 2 cents.
>=20
> Cheers,
>=20
> f
>=20

Baptiste Carvello wrote:
> Hello,
> 
> here is an imporved version of the patch (minor bug corrected + diffed 
> against latest CVS).
Just a note: are you aware of the gui_thread module in scipy? It was written 
exactly to do this (though it's currently WX-specific, I think), and it has 
seen a pretty extensive amount of work. I would much rather see a generic, 
external solution be tested and improved than a matplotlib-specific rather 
intrusive patch. Don't take it badly, I can tell you've put a lot of work 
into this, but this kind of problem is IMHO best addressed by a more generic 
mechanism. The kind of attribute access which python allows opens the door 
for such a solution, as gui_thread has shown.
gui_thread currently needs testing and improvements, but I think in the long 
run it is a cleaner, better alternative.
Just my 2 cents.
Cheers,
f

Hello,
here is an imporved version of the patch (minor bug corrected + diffed 
against latest CVS).
By the way, it looks like the web interface is cutting off the patch. 
Probably the right thing given its size. Providing a link to the 
attached file would be nicer, though...
Cheers,
BC

Fernando Perez wrote:
> Just a minor comment: insert(0) is an expensive (O(N)) operation on lists. 
> This kind of problem might be better addressed with a deque 
> (http://www.python.org/dev/doc/devel/lib/module-collections.html, could be 
> backported to py2.3).
Sorry for the extra msg, here's a pure python backport of deque by R. 
Hettinger, the author of the C collections module in py2.4:
http://aspn.activestate.com/ASPN/Cookbook/Python/Recipe/259179
Cheers,
f

Hello,
here is the corrected patch, sorry for the noise.
Cheers,
BC

Joe wrote:
> Hello John and other devels,
> 
> John, you were right on the money in relation to the memory leak that I wrote 
> about in the previous message. Here's a rough draft of an attempt to patch 
> that up. This is my first (meaningful) attempt to submit a patch anywhere, 
> so please forgive any errors in the process. It was made with >patch -p ... 
> I think that is the accepted way to make a patch, but I'm not sure.
> *** text.py	Tue Jan 18 21:03:16 2005
> --- /root/text.py	Tue Jan 18 20:58:56 2005
> *************** class Text(Artist):
> *** 44,49 ****
> --- 44,50 ----
> 
> Artist.__init__(self)
> self.cached = {}
> + 	self.usage_stack = []
> self._x, self._y = x, y
> 
> if color is None: color = rcParams['text.color']
> *************** class Text(Artist):
> *** 193,198 ****
> --- 194,203 ----
> 
> ret = bbox, zip(lines, whs, xs, ys)
> self.cached[key] = ret
> + 	self.usage_stack.insert(0, key)
> + 	if len(self.usage_stack) > 128:
> + 	 self.usage_stack.pop()
> + 	 del self.cached[key]
> return ret
Just a minor comment: insert(0) is an expensive (O(N)) operation on lists. 
This kind of problem might be better addressed with a deque 
(http://www.python.org/dev/doc/devel/lib/module-collections.html, could be 
backported to py2.3).
Cheers,
f

Hello John and other devels,
John, you were right on the money in relation to the memory leak that I wrote 
about in the previous message. Here's a rough draft of an attempt to patch 
that up. This is my first (meaningful) attempt to submit a patch anywhere, 
so please forgive any errors in the process. It was made with >patch -p ... 
I think that is the accepted way to make a patch, but I'm not sure.
At first I wanted to plug this up by resuing the "cached" variable (by 
removing the last key from the .keys() method if the number of entries got 
too big), but I didn't like how that turned out so I decided to add a list 
which was sorted by last use. Removal when there became too many items was 
performed on the least recently used entry. I think that this way turned out 
for the better.
What might improve things is if there was a way to check how big the data 
structures which we are caching are in memory. The first thought that came 
to my mind was to dump the structures to a Python string via cPickle and 
count the bytes used, but this would be too slow I think. At the very least, 
the value could be supplied by the .matplotlibrc file, but I'm not very sure 
where all those values are read in.
Anyways too much talk for such a small fix,
Take care and happy coding,
Joe

Hello,
I got confused with my two patches. The pylab.py chunk from the other 
patch actually belongs here.
Sorry for that, I'll send a corrected version tomorrow, I need some sleep :)
Cheers,
BC