MEP27 Decouple pyplot from backends (refactoring Gcf out of backend code) #4143
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -385,7 +385,7 @@ def stop_event_loop(self): | |
| FigureCanvasBase.stop_event_loop_default(self) | ||
| stop_event_loop.__doc__=FigureCanvasBase.stop_event_loop_default.__doc__ | ||
|
|
||
| class Window(WindowBase, Gtk.Window): | ||
|
There was a problem hiding this comment. Why don't keep the name something like There was a problem hiding this comment. Personally, I don't really see what difference it makes apart from taking up extra hard drive and memory space. At the moment we have I don't mind renaming it, I just don't see any need for it... There was a problem hiding this comment. I kind of agree with you, but in that case, why don't we remove all the There was a problem hiding this comment. 👍 On making the GUI specific classes have the framework name in them and doing There was a problem hiding this comment. I guess "bad" beats "ugly" (ugly in terms of a list of housekeeping aliases at end of each file. I can't imagine how it could get bad, (on the other hand I found debugging that transform logic a nightmare with multiple layers of transforms to go through and understand what they what they were doing and why they were failing), so meh. |
||
| def __init__(self, title): | ||
| WindowBase.__init__(self, title) | ||
| Gtk.Window.__init__(self) | ||
|
|
@@ -403,22 +403,35 @@ def __init__(self, title): | |
| # better way is - JDH | ||
| verbose.report('Could not load matplotlib icon: %s' % sys.exc_info()[1]) | ||
|
|
||
| self._layout = {} | ||
| self._setup_box('_outer', Gtk.Orientation.VERTICAL, False, None) | ||
| self._setup_box('north', Gtk.Orientation.VERTICAL, False, '_outer') | ||
|
There was a problem hiding this comment. I find it a little bit strange the nomenclature There was a problem hiding this comment. Meh, the first programming language I learnt, Java, used this nomenclature, so does jquery (for websites, i.e. I don't mind which nomenclature, just so long as we stay clear of Parkinson's law 😉. Also just found this (you don't need to look at it, but just leaving it here for future reference) which should help with Qt http://ftp.ics.uci.edu/pub/centos0/ics-custom-build/BUILD/PyQt-x11-gpl-4.7.2/examples/layouts/borderlayout.py There was a problem hiding this comment. Yep, it's only me, don't worry, leave it like that There was a problem hiding this comment. Of course I only gave examples of where this nomenclature gets used, so interpret the above as having a strong confirmation bias, but I guess you don't mind either way. |
||
| self._setup_box('_middle', Gtk.Orientation.HORIZONTAL, True, '_outer') | ||
| self._setup_box('south', Gtk.Orientation.VERTICAL, False, '_outer') | ||
|
|
||
| self._setup_box('west', Gtk.Orientation.HORIZONTAL, False, '_middle') | ||
| self._setup_box('center', Gtk.Orientation.VERTICAL, True, '_middle') | ||
| self._setup_box('east', Gtk.Orientation.HORIZONTAL, False, '_middle') | ||
|
|
||
| self.add(self._layout['_outer']) | ||
|
|
||
| self.connect('destroy', self.destroy_event) | ||
| self.connect('delete_event', self.destroy_event) | ||
|
|
||
| def _setup_box(self, name, orientation, grow, parent): | ||
| self._layout[name] = Gtk.Box(orientation=orientation) | ||
| if parent: | ||
| self._layout[parent].pack_start(self._layout[name], grow, grow, 0) | ||
| self._layout[name].show() | ||
|
|
||
| def add_element_to_window(self, element, expand, fill, pad, place): | ||
| element.show() | ||
| if place in ['north', 'west', 'center']: | ||
| self._layout[place].pack_start(element, expand, fill, pad) | ||
| elif place in ['south', 'east']: | ||
| self._layout[place].pack_end(element, expand, fill, pad) | ||
| else: | ||
| raise KeyError('Unknown value for place, %s' % place) | ||
| size_request = element.size_request() | ||
| return size_request.height | ||
|
|
||
|
|
@@ -430,7 +443,6 @@ def show(self): | |
| Gtk.Window.show(self) | ||
|
|
||
| def destroy(self): | ||
| Gtk.Window.destroy(self) | ||
|
|
||
| def set_fullscreen(self, fullscreen): | ||
|
|
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 like it, but, as you said in the comment, how many of our current backends (GUI backends) support all the options?
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.
Not sure, I think I want to get rid of
padas I think we always want that as 0? Also eitherfillorexpandas I feel we will always have them both asTrueor both asFalse, and feels like extra work to research if methods exist to do it for the different backends.Also what do you think about renaming this method, to just
add_elementoradd_widget, I think_to_windowbecomes extraneous, as it gets called on theWindowclass, but I can't decide between the two names...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 would change it to
add_widgetThere 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.
Are these native GUI widgets or mpl widgets?
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.
These are the GUI widgets that are going to be defined for each backend, so far
Window,Toolbar,MessageThere 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.
@OceanWolf yes I mean
Statusbar, but you have to removeNavigationToolbar2that I believe it's not compatible with this approachThere 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.
@fariza With which part of this approach? It works fine for me...
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 mean that I am not sure
NavigationToolbar2will work in all the backends, as I am not sure they all return just the widget. But it will be matter of testing.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.
Ahh, well I have gone through all of the GUI backends apart from the Mac specific ones 😒 and GTK2, prior to MEP22 going to master, and got them all to work.
Though if I remember correctly I did have to do this on one them:
but that doesn't matter as we can do that kind of thing on the specific backend 😉.
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.
okay, so how about
add_element(self, element, expand, place)?