silx.gui.plot.PlotWidget: Added yaxis arcsinh support with matplotlib backend#4306
silx.gui.plot.PlotWidget: Added yaxis arcsinh support with matplotlib backend#4306EdgarGF93 wants to merge 5 commits intosilx-kit:mainfrom
Conversation
|
Nice. This could also be a good opportunity to rework the backend API with normalization. Because actually, to set linear, we have to set |
|
I think I'll leave the opengl for somebody else :) |
t20100
changed the title
t20100
left a comment
t20100
left a comment
There was a problem hiding this comment.
Let's OpenGL support for later.
I also propose to do this PR on adding arcsinh to PlotWidget with matplotlib and keep the QAction for a second one.
Could you add arcsinh scale for x axis as well for consistency?
| self._plotFrame.y2Axis.isLog = flag | ||
|
|
||
| def setYAxisArcsinh(self, flag): | ||
| ... |
There was a problem hiding this comment.
| ... | |
| raise NotImplementedError("Plot OpenGL backend does not support arcsinh Y axis") |
| </g> | ||
| <g id="logGroup"> | ||
| </g> | ||
| <text id="asinhText" x="8.0" y="19" font-weight="bold" font-family="sans-serif" font-size="0.56em">asinh</text> |
There was a problem hiding this comment.
Could you convert the text to a path so the icon does not rely on system fonts?
If you don't know how to do it, i can show you.
| def setYAxisArcsinh(self, flag): | ||
| if flag: | ||
| self.ax2.set_yscale("asinh") | ||
| self.ax.set_yscale("asinh") | ||
| return | ||
|
|
||
| self.ax2.set_yscale("linear") | ||
| self.ax2.yaxis.set_major_formatter(DefaultTickFormatter()) | ||
| self.ax.set_yscale("linear") | ||
| self.ax.yaxis.set_major_formatter(DefaultTickFormatter()) |
There was a problem hiding this comment.
Would be best to change the backend to have a single place where to set scale for yaxis.
I would replace setYAxisLogarithmic and setYAxisArcsinh by a setScale.
The backend are not marked as private with an _ but they should have and we don't keep the compatibility.
| def _internalSetArcsinh(self, flag): | ||
| self._getBackend().setYAxisArcsinh(flag) |
There was a problem hiding this comment.
Same as above, _internalSetLogarithmic and _internalSetArcsinh could be replaced by a _internalSetScale
| self.yAxisArcsinhAction = self.group.addAction( | ||
| actions.control.YAxisArcsinhAction(self, parent=self) | ||
| ) | ||
| self.yAxisArcsinhAction.setVisible(logScale) | ||
| self.addAction(self.yAxisArcsinhAction) |
There was a problem hiding this comment.
Adding a new axis scaling changes its control from a boolean to an enum (it should have been so since the beginning).
It would make sense to me to replace yAxisLogarithmicAction with a toolbutton with a drop-down.
3 participants
Checklist:
<Module or Topic>: <Action> <Summary>(see contributing guidelines)