MEP27 Decouple pyplot from backends (refactoring Gcf out of backend code) #4143
Conversation
|
@tacaswell Not finished yet, still some refactoring to go, just putting the PR for early feedback to catch things that I have perhaps missed. I have finished the first pass (out of 2, I hope), so it should all work. I have worked solely on the |
|
Atm, I use the existing structure, i.e. using from matplotlib.backends import Window, Toolbar2, Canvasand in _temp = __import__('backend_'+backend, globals(), locals(), ['Window', 'Toobar2', 'Canvas'], -1)
Window = _temp.Window
Toolbar2 = _temp.Toolbar2
Canvas = _temp.Canvas |
|
I provisionally think this is a good idea. This deserves more attention than I can give it right now, sorry 😞 Would this method also provide a (sane) path to change which interactive backend you are using on the fly? I don't think it will, but make sure that this does not interfere with how we do saving (which is (more-or-less) by replacing the canvas on the |
|
Thank you, I shall push ahead then with the additions to Perhaps not a sane path, (at least not right now), but I guess it will provide a saner path for later (as with this we get some clear structure to how the backends work). I have never tried to change backends and so don't know the current path, so I don't really know. I imagine that with the additions to |
|
@tacaswell Almost finished, just finishing the refactoring Gcf out of One question arises though, should the If show should only get called via Gcf, then I shall probably create a method |
|
I mean I feel 95% sure, that the backend Okay, I think have talked myself round to believing my assumption as correct and will give it a try. If I have overlooked something here please let me know! |
d822f38 to
7dba22f
Compare
2ddb536 to
1f7e9f4
Compare
|
Okay, so I have finished the 2nd refactor pass so open for comments, I have tested I did have problems with absolute imports due to Will feel good to get this landed as it will eradicate the backend hacks from #3652. Of course I imagine changes to |
94b7bce to
3cfaec1
Compare
|
Yay Travis working again, but boo failing, did I introduce an error here, or does this represent a problem with the testing framework? Looking at the log for 2.6 I get the following, and the |
ceec49f to
61a7090
Compare
|
Okay, now that Travis gives the thumbs up to this PR, I now see it time to start pushing out to other backends. The strategy for this:
|
22c9a39 to
798a62a
Compare
|
@tacaswell Things look good, I should have all the backends (apart from the two mac ones) converted by the end of the weekend. I don't use macs, only linux and sparingly win. Does it matter if they don't get converted right now? Or I could try and code it blind. Also anything I should do to ease the review? @fariza suggested opening an MEP for this. |
|
Gah, help, why does this not build? I cannot see what I have changed since the last okay from travis that could have caused this build doc failiure... The build gives the following as one line. |
|
I think you need to escape the * in that doc string TEXT is default for emphasising text in RST so it complains that you start an emphasised section but not end it. See http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#emphasis A backslash should do |
ea9f1e4 to
a0d1110
Compare
|
Thanks @jenshnielsen , that has fixed it :D. On a side note, when I run |
|
Ok, now that is weird. surface3d_demo.py explicitly computes its own data: That is always 2d, no matter what. It would be helpful to know exactly On Tue, Mar 3, 2015 at 12:03 PM, OceanWolf notifications@github.com wrote:
|
|
I think I have seen that error when building the docs. It happened because I had a leftover old version of mpl_toolkits in my sitepackages folder from before everything was moved to the matplotlib folder. |
e7290fe to
64f0c61
Compare
|
Define nights... the international nature of this project means we should discuss times in UTC (Universal Coordinated Time). |
|
In that case I'm UTC-4 So the best for me is after UTC 1:00am On Aug 9, 2016 8:15 AM, "OceanWolf" notifications@github.com wrote:
|
| self._is_gui = hasattr(self._backend, 'Window') | ||
| if not self._is_gui: | ||
| self.window = None | ||
| return |
There was a problem hiding this comment.
I don't think return here is going to work (init returns the object not a value/None).
What about moving the gui setup (window, toolbar, ....) to another method, that doesn't get called if is not gui.?
There was a problem hiding this comment.
give me copy paste code to show that it doesn't work... __init__ works as a normal method, i.e. it has an implicit return, i.e. return None at the end...
I think you get confused with __new__ as new does that... not sure what object.new looks like but you get the gist from this:
def __new__(class, *args, **kwargs):
instance = super().__new__(class, *args, **kwargs) # calls __init__(self, *args, **kwargs)
return instancebasically:
- From the code
MyClass(*args, **kwargs)
2.__new__(MyClass, *args, **kwargs)gets called. - That magically creates the instance and calls
__init__on it. - Returns the instance.
There was a problem hiding this comment.
I got confused because init must return None.
I just remembered that we shouldn't return things from init ;)
There was a problem hiding this comment.
yup, because it happens with __new__, you can do fun things with __new__...
class NaughtyBoy(object):
def __init__(self):
print('Just a naughty boy')
class Messiah(object):
def __init__(self):
print('Hallelujah!')
def __new__(cls, *args, **kwargs):
print('Not the Messiah')
return NaughtyBoy()
person = Messiah()
print('I wanted the Messiah, but I got a', type(person))|
At the moment I work on UTC+2, so a bit difficult for us arrange a time... I guess I could work a night... |
|
Weekend before UTC 2pm? On Aug 15, 2016 6:37 PM, "OceanWolf" notifications@github.com wrote:
|
|
@tacaswell @mdboom @OceanWolf can we look for a time and date to take a look at this PR? I understand that there might be problems/corrections with the specific implementation, that is something we can work on. |
|
@tacaswell Can we include this in one of the weekly phone calls? |
|
A couple weeks ago we discussed this on the Monday conference and agreed that this work is the primary target for 2.1, so it has high priority as soon as 2.0 is out. |
|
Great! On Oct 22, 2016 4:17 PM, "Eric Firing" notifications@github.com wrote:
|
|
I have rebased this with the same name in my repo. |
|
Hello everyone
Can we plan sometime for reviewing?
Thanks
Federico
…On Feb 11, 2017 7:00 PM, "Thomas A Caswell" ***@***.***> wrote:
I have rebased this with the same name in my repo.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4143 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABa86XPfk13LS9YaOROLLxsAuQMqgHHEks5rbkuugaJpZM4Dj1l2>
.
|
|
I will likely have time to look at this again in a couple of weeks.
On Wed, Apr 26, 2017 at 9:16 AM, Federico Ariza <notifications@github.com>
wrote:
… Hello everyone
Can we plan sometime for reviewing?
Thanks
Federico
On Feb 11, 2017 7:00 PM, "Thomas A Caswell" ***@***.***>
wrote:
> I have rebased this with the same name in my repo.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/matplotlib/matplotlib/pull/
4143#issuecomment-279185227>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/
ABa86XPfk13LS9YaOROLLxsAuQMqgHHEks5rbkuugaJpZM4Dj1l2>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4143 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AARy-EKK4o_AfokYxk4SrG1HD_3J4ww5ks5rz0PAgaJpZM4Dj1l2>
.
|
|
If you think it could help.
We can do a couple of hangout sessions after you go through the code.
Federico
…On Apr 27, 2017 3:21 PM, "Benjamin Root" ***@***.***> wrote:
I will likely have time to look at this again in a couple of weeks.
On Wed, Apr 26, 2017 at 9:16 AM, Federico Ariza ***@***.***>
wrote:
> Hello everyone
>
> Can we plan sometime for reviewing?
>
> Thanks
> Federico
>
> On Feb 11, 2017 7:00 PM, "Thomas A Caswell" ***@***.***>
> wrote:
>
> > I have rebased this with the same name in my repo.
> >
> > —
> > You are receiving this because you were mentioned.
> > Reply to this email directly, view it on GitHub
> > <https://github.com/matplotlib/matplotlib/pull/
> 4143#issuecomment-279185227>,
> > or mute the thread
> > <https://github.com/notifications/unsubscribe-auth/
> ABa86XPfk13LS9YaOROLLxsAuQMqgHHEks5rbkuugaJpZM4Dj1l2>
>
> > .
> >
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/matplotlib/matplotlib/pull/
4143#issuecomment-297404118>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AARy-EKK4o_
AfokYxk4SrG1HD_3J4ww5ks5rz0PAgaJpZM4Dj1l2>
> .
>
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#4143 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABa86XQzvvBhBIhOTyViLmOQ5DdsPYuMks5r0OqzgaJpZM4Dj1l2>
.
|
|
Still out of it for a bit, hope to get back in to the groove soon. |
|
Instead of MainLoop.begin and MainLoop.end, I think it may be more pythonic to make MainLoop a contextmanager ( |
This PR exists as an alternative to #2624
See https://github.com/OceanWolf/matplotlib/blob/backend-refactor/doc/devel/MEP/MEP27.rst for details.
Tl;dr:
FigureManagerBase➡️FigureManager+WindowBaseShowBase➡️Gcf.show_all+MainLoopBaseThe main refactor (+gtk3 as an example) goes here, and will have no effect on the individual backends, work for other backends will occur in separate branches as indicated.
MacOSXGTKGTKCairoGTKAggNbAggWebAggOceanWolf/matplotlib@backend-refactor...OceanWolf:backend-refactor-webagg (still W-I-P, but will deal with last items in its PR)WxOceanWolf/matplotlib@backend-refactor...backend-refactor-wxWxAggQtOceanWolf/matplotlib@backend-refactor...OceanWolf:backend-refactor-qtQt4AggQt5AggTkAggOceanWolf/matplotlib@backend-refactor...OceanWolf:backend-refactor-tkaggGTK3in this branchGTK3CairoGTK3AggTo accomplish this, we shall do this in two passes:
gtk3backend, replacing these with common methods, i.e. we refactor outGcftobackend_bases.py.Gcfout frombackend_bases.py.