Handle axis SI prefix scaling in MatplotlibExporter #1282
Conversation
|
FYI we can easily add matplotlib to our CI suite if you think this should be tested |
|
Should probably have a skip if decorator for skipping the test if matplotlib isn't installed |
|
I think it's probably worth adding some test coverage. I'm not sure how much we could/should test in the case of |
|
Sounds good, I'm not at a computer right now, do you need help with wing matplotlib to the CI pipelines? |
|
Was about to say nah I got it but of course there's a failure... I think I can figure it out but chime in if you notice something. At the moment I see the naive approach of adding matplotlib to the dependency install line pulled in (py)qt-5 in the Python2-PySide-4.8 job. |
|
Those Qt4/PySide1/Python2 configs are no joke. In the "install dependencies" step of the linux/pyside1 pipeline I won't be able to help debug this for a few hours; but the problem seems to be that installing matplotlib via conda bundles pyqt5 along with it; which is not at all what we want... I think we may want to install matplotlib via |
|
Wow, only the Linux Python2-PySide-4.8 job failed despite (py)qt-5 being installed for Windows/MacOS too. Well, I have to say I was afraid of pretty much exactly this. There's probably a way to write the requirement spec to avoid it, but I'm pretty sure that would ugly up the CI template quite a bit. @j9ac9k what do you think about only installing it for the pip jobs and letting pytest skip the tests otherwise (skipping alrady implemented)? I guess it would be nice to test this with a variety of Qt versions particularly because it's matplotlib with Qt backends though. edit: just saw your comment above - that sounds like a reasonable way to do it |
|
I think we can make this work by installing via pip and specifying which matplotlib dependencies we install. I'm definitely 👎 on writing spec files. I think either way we should use the skipif decorator and skip those tests if matplotlib is not installed |
|
So just installing with pip in the conda environments seems to work. Their setup.py makes no assumptions about what backend you want, so I think this is a pretty stable solution. The whole test module is skipped with Going to move this out of draft but feedback still welcome obviously. |
|
This might be outside of the scope of this PR, but does matplotlib qt widget offer any kind of dark mode recognition? on macOS, which is running dark mode, this is what the window looks like:
pyqtgraph does have some dark mode recognition in the codebase that was introduced as part of #1124 in class App(QtGui.QApplication):
def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
self.paletteChanged.connect(self.onPaletteChange)
self.onPaletteChange(self.palette())
def onPaletteChange(self, palette):
self.dark_mode = palette.base().color().name().lower() != "#ffffff" |
|
Some googling turns up this issue: A bit more googling, and light icons were merged, but they are slated to be in the 3.3.0 release! I'm good with hoping this sorts itself out for matplotlib 3.3.0 |
Finally following up #1051
Basic test:
I ended up sticking with the basic idea of #1051, keeping the axis labels and scaling the data. The alternative I had considered was stripping the prefix from the label and letting matplotlib handle the axis scaling with a multiplier label, but this maintains the pyqtgraph style.
I recommend viewing the diff ignoring whitespace changes. I usually try to make minimal diffs, but removing the unnecessary indentation level seemed worthwhile.
No test because we don't have matplotlib in CI afaik.Fixes #1050
Fixes #1051