Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
MEP27 Decouple pyplot from backends (refactoring Gcf out of backend code)#4143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
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 frommatplotlib.backendsimportWindow,Toolbar2,Canvas and in _temp=__import__('backend_'+backend,globals(),locals(), ['Window','Toobar2','Canvas'],-1)Window=_temp.WindowToolbar2=_temp.Toolbar2Canvas=_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 to7dba22fCompare2ddb536 to1f7e9f4CompareOkay, 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 to3cfaec1CompareYay 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 to61a7090CompareOkay, 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 to798a62aCompare@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 stringTEXT is default for emphasising text in RST so it complains that you start an emphasised section but not end it. Seehttp://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#emphasis A backslash should do |
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, OceanWolfnotifications@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. |
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
give me copy paste code to show that it doesn't work...__init__ works as a normal method, i.e. it has an implicitreturn, 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)returninstance
basically:
- 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
yup, because it happens with__new__, you can do fun things with__new__...
classNaughtyBoy(object):def__init__(self):print('Just a naughty boy')classMessiah(object):def__init__(self):print('Hallelujah!')def__new__(cls,*args,**kwargs):print('Not the Messiah')returnNaughtyBoy()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 everyoneCan we plan sometime for reviewing?ThanksFederico …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 ( |
Uh oh!
There was an error while loading.Please reload this page.
This PR exists as an alternative to#2624
Seehttps://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.