Use napari_scraper instead of qtgallery #207
Conversation
github-actions
bot
added
the
documentation
|
OMG! The CircleCI preview has the full gallery! |
|
On mac I'm getting the following warnings: This also breaks the corresponding example outputs. Using time, I have the following on main: And for this PR CircleCI output looks good though - so this is probably mac specific |
|
Weird, I'm not building docs, but doing: works on my macOS. |
|
Weird - this actually fixed building the docs locally on my Mac but I thought maybe it was a side-effect or something else I did. I'll try to start with a fresh env. I've been using pyqt6 but will try with other backends. |
|
Here's my conda env if that's helpful: (note that I'm using pyqt5 as I'm doing EDIT: PyQT6 doesn't solve it unfortunately |
|
Thanks - your env looks fine to me. I just remembered though that I think I had this error before and made a change in my (local) napari to fix it. Are you able to build docs with I was having a problem with how sphinx-gallery tries to do...something...with the imports and it was breaking when combined with the way napari does some lazy importing on the main module. The symptom was that every example in the gallery after This is how I worked around it: ❯ git diff main -- examples/custom_key_bindings.py
diff --git a/examples/custom_key_bindings.py b/examples/custom_key_bindings.py
index f78ffe53b..90ece4add 100644
--- a/examples/custom_key_bindings.py
+++ b/examples/custom_key_bindings.py
@@ -11,6 +11,8 @@ from skimage import data
import napari
+Viewer = napari.Viewer
+
blobs = data.binary_blobs(
length=128, blob_size_fraction=0.05, n_dim=2, volume_fraction=0.25
).astype(float)
@@ -41,7 +43,7 @@ def set_layer_data(viewer):
viewer.layers[0].data = blobs
-@napari.Viewer.bind_key('w')
+@Viewer.bind_key('w')
def hello(viewer):
# on press
viewer.status = 'hello world!'(this also took me a really long time to debug/workaround) edit: all that said I just created a new env and it is now working with |
|
I am able to reproduce the |
|
That did fix it! Any ideas why? I've been building like this forever because I am often changing things in both napari and the docs at the same time, so understanding that would also be helpful... |
Sorry, but not really :( - I don't know enough about the differences between editable and "normal" installs to say (and I think it's even changed a bit in the past couple years). I chased it a bit and found sphinx-gallery was doing some interesting stuff to try to figure out where different imports in the examples come from, I think as part of looking for backreferences. In any case this caused |
|
Thanks - that is helpful to know and maybe should be documented in our development docs so I'll open a PR for that. Cheers! EDIT: here it is #209 |
|
I ❤️ 😍 this! Thank you!!! 🙏 |
The backreferences code in Sphinx Gallery is very complex and confusing, but I really don't think it executes anything. It parses the code into AST but nothing more. And I just checked and we do not set even I am intrigued as to why |
|
I am not sure if this will make a difference, but would directly importing Edit2: Actually would doing (Sorry I tried to test on the mac, but its updating and downloading and estimating to take hours) |
I think it would be safer to add this to the reset function. Potentially it may remove references to things that will enable garbage collection and aid memory usage. Will open a PR for this. Edit: Technically it is probably fine for it to be in the scraper as in our case the scraper is always run when executing examples, so will leave as is. |
Yes I think doing some import tricks may prevent the error, but as you note it's more of a band-aid or workaround. I think it's ultimately caused by the way things are imported in one or more examples ( I will continue digging into this because it's a sufficiently interesting (annoying) mystery.
It does feel more like reset-type functionality, so I'll also play with this. I'm glad this PR was merged quickly to get CI green but it's probably not the last time we'll have to touch it 😄. |
# Description This is a CI change that removes `qtgallery` in favor of a napari-specific scraper. It's not that different but gives us a little more control and is not much code. This seems to fix the error seen in "Build PR Docs" for napari/napari#4865 and also speeds up the docs build quite a bit (~11 min instead of ~25 min). I'm no Qt expert but suspect the main improvements here are related to adding `napari.Viewer.close_all()` (which maybe belongs in the reset fn) and calling `processEvents()` one more time after this. The main drawback right now is that this doesn't capture non-Viewer windows, but this could probably be added if needed. ## Type of change - [x] Fixes or improves workflow, documentation build or deployment # References closes napari#174 (maybe?) fixes errors in docs build for napari/napari#4865 ## Final checklist: - [x] My PR is the minimum possible work for the desired functionality - [x] I have commented my code, particularly in hard-to-understand areas
Description
This is a CI change that removes
qtgalleryin favor of a napari-specific scraper. It's not that different but gives us a little more control and is not much code.This seems to fix the error seen in "Build PR Docs" for napari/napari#4865 and also speeds up the docs build quite a bit (~11 min instead of ~25 min).
I'm no Qt expert but suspect the main improvements here are related to adding
napari.Viewer.close_all()(which maybe belongs in the reset fn) and callingprocessEvents()one more time after this.The main drawback right now is that this doesn't capture non-Viewer windows, but this could probably be added if needed.
Type of change
References
closes #174 (maybe?)
fixes errors in docs build for napari/napari#4865
Final checklist: