Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Simplify and improve Qt borders/spacing tool.
Replaced sliders in Qt borders/spacing tool by spinboxes, which
  1. should be easier to set to an exact value, and
  2. do not continuously trigger redraws unless the user presses enter
     or uses the arrows to step the values (the redraws can be quite
     slow when working with a complex plot).

The spinbox step size of 0.005 was chosen for consistency with the
earlier choice of 5/1000.

Greatly simplified the implementation.  Attributes on the SubplotToolQt
instance are not kept because it is impossible to keep
back-compatibility (the sliders simply don't exist anymore).  New
attributes are all private; only `.default` (which has the same meaning)
is kept as is.

Tested with PyQt 4.12, PySide 1.2.4, PyQt 5.8.
  • Loading branch information
anntzer committed May 29, 2017
commit 244a3e0097983ab94bf1be1ece98b6a082374447
1 change: 0 additions & 1 deletion lib/matplotlib/backends/backend_qt4.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@
from matplotlib.widgets import SubplotTool

from .qt_compat import QtCore, QtWidgets, _getSaveFileName, __version__
from matplotlib.backends.qt_editor.formsubplottool import UiSubplotTool

from .backend_qt5 import (backend_version, SPECIAL_KEYS, SUPER, ALT, CTRL,
SHIFT, MODIFIER_KEYS, fn_name, cursord,
Expand Down
126 changes: 38 additions & 88 deletions lib/matplotlib/backends/backend_qt5.py
Original file line number Diff line number Diff line change
Expand Up @@ -782,97 +782,47 @@ class SubplotToolQt(SubplotTool, UiSubplotTool):
def __init__(self, targetfig, parent):
UiSubplotTool.__init__(self, None)

self.targetfig = targetfig
self.parent = parent
self.donebutton.clicked.connect(self.close)
self.resetbutton.clicked.connect(self.reset)
self.tightlayout.clicked.connect(self.functight)

# constraints
self.sliderleft.valueChanged.connect(self.sliderright.setMinimum)
self.sliderright.valueChanged.connect(self.sliderleft.setMaximum)
self.sliderbottom.valueChanged.connect(self.slidertop.setMinimum)
self.slidertop.valueChanged.connect(self.sliderbottom.setMaximum)

self.defaults = {}
for attr in ('left', 'bottom', 'right', 'top', 'wspace', 'hspace', ):
val = getattr(self.targetfig.subplotpars, attr)
self.defaults[attr] = val
slider = getattr(self, 'slider' + attr)
txt = getattr(self, attr + 'value')
slider.setMinimum(0)
slider.setMaximum(1000)
slider.setSingleStep(5)
# do this before hooking up the callbacks
slider.setSliderPosition(int(val * 1000))
txt.setText("%.2f" % val)
slider.valueChanged.connect(getattr(self, 'func' + attr))
self._setSliderPositions()

def _setSliderPositions(self):
for attr in ('left', 'bottom', 'right', 'top', 'wspace', 'hspace', ):
slider = getattr(self, 'slider' + attr)
slider.setSliderPosition(int(self.defaults[attr] * 1000))

def funcleft(self, val):
if val == self.sliderright.value():
val -= 1
val /= 1000.
self.targetfig.subplots_adjust(left=val)
self.leftvalue.setText("%.2f" % val)
if self.drawon:
self.targetfig.canvas.draw_idle()

def funcright(self, val):
if val == self.sliderleft.value():
val += 1
val /= 1000.
self.targetfig.subplots_adjust(right=val)
self.rightvalue.setText("%.2f" % val)
if self.drawon:
self.targetfig.canvas.draw_idle()

def funcbottom(self, val):
if val == self.slidertop.value():
val -= 1
val /= 1000.
self.targetfig.subplots_adjust(bottom=val)
self.bottomvalue.setText("%.2f" % val)
if self.drawon:
self.targetfig.canvas.draw_idle()

def functop(self, val):
if val == self.sliderbottom.value():
val += 1
val /= 1000.
self.targetfig.subplots_adjust(top=val)
self.topvalue.setText("%.2f" % val)
self._figure = targetfig

for lower, higher in [("bottom", "top"), ("left", "right")]:
self._widgets[lower].valueChanged.connect(
lambda val: self._widgets[higher].setMinimum(val + .001))
self._widgets[higher].valueChanged.connect(
lambda val: self._widgets[lower].setMaximum(val - .001))

self.defaults = {
attr: getattr(self._figure.subplotpars, attr)
for attr in ["left", "bottom", "right", "top", "wspace", "hspace"]}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest making this dictionary generation a method of Figure.SubplotParams, so the line would become, e.g.,

self.defaults = self._figure.subplotpars.dict()

# Set values after setting the range callbacks, but before setting up
# the redraw callbacks.
self._reset()

for attr in self.defaults:
self._widgets[attr].valueChanged.connect(self._on_value_changed)
for action, method in [("Tight Layout", self._tight_layout),
("Reset", self._reset),
("Close", self.close)]:
self._widgets[action].clicked.connect(method)

def _on_value_changed(self):
self._figure.subplots_adjust(
**{attr: self._widgets[attr].value() for attr in self.defaults})
if self.drawon:
self.targetfig.canvas.draw_idle()

def funcwspace(self, val):
val /= 1000.
self.targetfig.subplots_adjust(wspace=val)
self.wspacevalue.setText("%.2f" % val)
if self.drawon:
self.targetfig.canvas.draw_idle()

def funchspace(self, val):
val /= 1000.
self.targetfig.subplots_adjust(hspace=val)
self.hspacevalue.setText("%.2f" % val)
self._figure.canvas.draw_idle()

def _tight_layout(self):
self._figure.tight_layout()
for attr in self.defaults:
widget = self._widgets[attr]
widget.blockSignals(True)
widget.setValue(getattr(self._figure.subplotpars, attr))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then I would probably use the new dict() method to make a dictionary after calling tight_layout, and use that dictionary to replace the getattr calls. This is a minor matter of taste, and I wouldn't insist on it, but to me it would be more pythonic. Regardless, having something like the dict() method available would be generally useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would vars(self._figure.subplotpars)[attr] be good enough for you?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've never used vars with an argument--I didn't know it worked that way. Now that I do, I would say that using it is indeed more readable than using the call to getattr, which I find rather ugly. Or one could use the equivalent self._figure.subplotpars.__dict__[attr]. Or SubplotParams could be turned into a specialized Bunch, so it could be indexed directly.

widget.blockSignals(False)
if self.drawon:
self.targetfig.canvas.draw_idle()

def functight(self):
self.targetfig.tight_layout()
self._setSliderPositions()
self.targetfig.canvas.draw_idle()
self._figure.canvas.draw_idle()

def reset(self):
self.targetfig.subplots_adjust(**self.defaults)
self._setSliderPositions()
self.targetfig.canvas.draw_idle()
def _reset(self):
for attr in self.defaults:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

self.defaults keeps being used simply as a source of the list of parameters; for the sake of readability, I might replace those uses with a genuine list, e.g. self.params. Yes, it would add one more line of code to the module.

self._widgets[attr].setValue(self.defaults[attr])


def error_msg_qt(msg, parent=None):
Expand Down
Loading