Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add sphinx gallery to docs #2323

Draft
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

ixjlyons
Copy link
Member

@ixjlyons ixjlyons commented Jun 5, 2022

https://pyqtgraph--2323.org.readthedocs.build/en/2323/examples/index.html

I'm thinking once this is working relatively well, we can start tackling some other improvements like categorizing examples in a separate PR.

- Add title to each example docstring
- Add examples README
- Sphinx conf is a total hack right now, need to figure out something
  better, potentially rename examples
Still seeing some "C++ object deleted..." errors, but at least with
PyQt6, these seem to not be fatal.
@ixjlyons ixjlyons marked this pull request as draft June 5, 2022 00:05
pyqtgraph/__init__.py Outdated Show resolved Hide resolved
@@ -4,7 +4,7 @@
not run twice.
"""

from ExampleApp import ExampleLoader
from _ExampleApp import ExampleLoader
Copy link
Member Author

@ixjlyons ixjlyons Jun 5, 2022

Choose a reason for hiding this comment

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

I guess I should check - this doesn't break one of the various ways of running the example application does it? This and some of the other file renames weren't strictly necessary but it made coming up with an ignore_pattern a little easier.

@@ -1,20 +1,25 @@
"""
Display a plot and an image with minimal setup.
CLI Example
Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Was wondering if this example even worked anymore now that interactive mode doesn't necessarily work with some bindings.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good point, looks like this usage currently only works with PyQt{5,6}. So maybe not the best example to iterate on, but I believe this would still work with PySide at least for the docs build. I use PySide2 for qtgallery and the iterative example works.

@ixjlyons
Copy link
Member Author

ixjlyons commented Jun 5, 2022

Playing with the CLI example, I found that with PyQt5, running that example interactively in a REPL, right clicking on the image, then exiting caused a RuntimeError: Internal C++ object... error from ViewBoxMenu. I'd seen some errors like this in the gallery build, so after some digging, I found that wrapping these two lines in a try / except RuntimeError got rid of it and building the docs with PyQt5 and PyQt6 gives no errors:

c = self.ctrl[i].linkCombo
current = c.currentText()

I'd need to understand a little better what's going on in ViewBoxMenu.setViewList (maybe some of the code after that should still run in some cases?), but this is promising.

I do still see a bunch of these types of errors with PySide:

Traceback (most recent call last):
  File "/home/krlyons/src/pyqtgraph/pyqtgraph/examples/DataSlicing.py", line 25, in <module>
    imv1 = pg.ImageView()
  File "/home/krlyons/src/pyqtgraph/doc/source/../../pyqtgraph/imageview/ImageView.py", line 129, in __init__
    self.ui.setupUi(self)
  File "/home/krlyons/src/pyqtgraph/doc/source/../../pyqtgraph/imageview/ImageViewTemplate_pyside6.py", line 42, in setupUi
    self.histogram = HistogramLUTWidget(self.layoutWidget)
  File "/home/krlyons/src/pyqtgraph/doc/source/../../pyqtgraph/widgets/HistogramLUTWidget.py", line 22, in __init__
    self.item = HistogramLUTItem(*args, **kargs)
  File "/home/krlyons/src/pyqtgraph/doc/source/../../pyqtgraph/graphicsItems/HistogramLUTItem.py", line 107, in __init__
    self.vb = ViewBox(parent=self)
  File "/home/krlyons/src/pyqtgraph/doc/source/../../pyqtgraph/graphicsItems/ViewBox/ViewBox.py", line 236, in __init__
    self.updateViewLists()
  File "/home/krlyons/src/pyqtgraph/doc/source/../../pyqtgraph/graphicsItems/ViewBox/ViewBox.py", line 1699, in updateViewLists
    nv = sorted(ViewBox.NamedViews.values(), key=view_key)
  File "/home/krlyons/src/pyqtgraph/doc/source/../../pyqtgraph/graphicsItems/ViewBox/ViewBox.py", line 1696, in view_key
    return (view.window() is self.window(), view.name)
RuntimeError: Internal C++ object (ViewBox) already deleted.

But I'm somewhat hopeful a similar fix could work and might end up getting rid of some of these errors that occasionally pop up in regular usage (or does that still actually happen?)

@ixjlyons
Copy link
Member Author

ixjlyons commented Jun 5, 2022

Re: comment above

Yep, wrapping this in a try/except gets rid of the rest of the errors for PyQt and PySide

nv = sorted(ViewBox.NamedViews.values(), key=view_key)

The difference here is that from the traceback I pasted above, this is happening during setup and not teardown, so this is probably not the right approach. Maybe there's something in qtgallery to fix in this case.

@j9ac9k
Copy link
Member

j9ac9k commented Jun 20, 2022

Hi @ixjlyons I just made some documentation changes which caused some merge conflicts, sorry about that!

Once addressed, would you mind re-running with error on warning enabled again? I'd like to try and replicate and solve those if possible!

@ixjlyons
Copy link
Member Author

Updated with master and resolved conflicts. It's running now with fail_on_warning: true though those warnings are generated regardless.

The last thing I had done was play around with the environment to see if I could get all the OpenGL examples working reliably. I'm not really sure what's wrong but I do get some GL errors locally too.

For the record, here's are the patches I mentioned above that give me an error-free build locally (aside from the OpenGL stuff):

diff --git a/pyqtgraph/graphicsItems/ViewBox/ViewBox.py b/pyqtgraph/graphicsItems/ViewBox/ViewBox.py
index 3ca843ce..8454b26c 100644
--- a/pyqtgraph/graphicsItems/ViewBox/ViewBox.py
+++ b/pyqtgraph/graphicsItems/ViewBox/ViewBox.py
@@ -1693,7 +1693,10 @@ class ViewBox(GraphicsWidget):
             return (view.window() is self.window(), view.name)
 
         ## make a sorted list of all named views
-        nv = sorted(ViewBox.NamedViews.values(), key=view_key)
+        try:
+            nv = sorted(ViewBox.NamedViews.values(), key=view_key)
+        except RuntimeError:
+            return
 
         if self in nv:
             nv.remove(self)
diff --git a/pyqtgraph/graphicsItems/ViewBox/ViewBoxMenu.py b/pyqtgraph/graphicsItems/ViewBox/ViewBoxMenu.py
index cf468ed3..90bd6314 100644
--- a/pyqtgraph/graphicsItems/ViewBox/ViewBoxMenu.py
+++ b/pyqtgraph/graphicsItems/ViewBox/ViewBoxMenu.py
@@ -236,8 +236,11 @@ class ViewBoxMenu(QtWidgets.QMenu):
             self.viewMap[name] = v
             
         for i in [0,1]:
-            c = self.ctrl[i].linkCombo
-            current = c.currentText()
+            try:
+                c = self.ctrl[i].linkCombo
+                current = c.currentText()
+            except RuntimeError:
+                continue
             c.blockSignals(True)
             changed = True
             try:

doc/requirements.txt Outdated Show resolved Hide resolved
@j9ac9k
Copy link
Member

j9ac9k commented Jun 21, 2022

Heh, macOS seemed to have an issue 😆

image

Also some more issues on macOS:

Qt api: PySide2
WARNING: Could not find Xvfb

Qt api should definitely be PyQt6 here (I'm an idiot, this was my own doing); also macOS does not have Xvfb, we probably should use it if we see it, but not try and force the issue if it is not present.

EDIT: the pyside2 bit may have been my mistake...working on sorting that out now. was my mistake 😬

@j9ac9k
Copy link
Member

j9ac9k commented Jun 21, 2022

this Runtime error that is being encountered is screaming race condition to me; I think it would be beneficial if we tracked down the source (whether it's in qtgallery or in pyqtgraph). I'll poke around some.

@j9ac9k
Copy link
Member

j9ac9k commented Jun 21, 2022

Also after a make clean when I rerun I get this error:

WARNING: Duplicate example file name(s) found. Having duplicate file names will break some links. List of files: ['/Users/ogi/Developer/pyqtgraph/doc/source/../../pyqtgraph/examples/py2exe/plotTest.py']

Looking at our examples, looks like they are going in specific directories; we should probably rename those files here, since they're examples, I'm not too worried about going through a deprecation dance.

@ixjlyons
Copy link
Member Author

Heh, macOS seemed to have an issue laughing

Hmm, not sure what to make of this. I suspect it's because QScreen.grabWindow supports grabbing contents that aren't part of the application - there's a note here about iOS at least.

The original motivation for using QScreen.grabWindow over QWidget.grab was to support QML applications in qtgallery. It may be worth reconsidering that switch or coming up with something better.

Also some more issues on macOS:

Qt api: PySide2
WARNING: Could not find Xvfb

also macOS does not have Xvfb, we probably should use it if we see it, but not try and force the issue if it is not present.

That's pretty much what is happening - it's a non-fatal warning.

However, I just noticed that disabling the virtual display, the renderings are mostly not the plot window contents but are seemingly random grabs of my display contents (behind the window?). Looking more at the grabWindow docs, there's a warning about X11, but it doesn't seem to be describing this situation exactly. Speaking of race condition...

this Runtime error that is being encountered is screaming race condition to me

Yep, haven't been able to track it down to this point.

doc/source/conf.py Outdated Show resolved Hide resolved
@j9ac9k
Copy link
Member

j9ac9k commented Jun 21, 2022

been poking around, in just about all cases, the ones that are raising errors are plots that have QTimers. I would guess what's happening is that the QTimer is still connected to the update methods, and that timer is still firing after qtgallery shuts down the QApplication. I'm trying to think of what would be a good way to "stop" the QTimer's in each of these examples before having qtgallery kill the QApplication instance.

@j9ac9k
Copy link
Member

j9ac9k commented Jun 21, 2022

Hmm, not sure what to make of this. I suspect it's because QScreen.grabWindow supports grabbing contents that aren't part of the application - there's a note here about iOS at least.

I don't think this PR should attempt to address this issue. Was mostly bringing it up because it surprised me.


try:
from pyqtgraph.metaarray import *
except:

Check notice

Code scanning / CodeQL

Except block handles 'BaseException'

Except block directly handles BaseException.
from pyqtgraph.Qt import QtWidgets

try:
from pyqtgraph.metaarray import *

Check notice

Code scanning / CodeQL

'import *' may pollute namespace

Import pollutes the enclosing namespace, as the imported module [pyqtgraph.metaarray](1) does not define '__all__'.
@@ -0,0 +1,39 @@
#!/usr/bin/python
Copy link
Member Author

@ixjlyons ixjlyons Sep 14, 2022

Choose a reason for hiding this comment

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

TODO: re-remove this example, here and in the example list in _utils

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants