Repository navigation
Conversation
<details><summary>Claude's draft</summary> Importing fastplotlib used to import `rendercanvas.auto` at module level (via `fastplotlib.utils.gui`), which runs the GUI backend selection and raises ImportError when no Qt, glfw or wx backend can be loaded. This happened even for applications that pass their own `canvas` and `renderer` to `Figure`. The auto selection (Qt priming, `rendercanvas.auto` import, Jupyter banner, Qt submodule imports) now runs on first access of `RenderCanvas`, `loop`, `GUI_BACKEND`, `IS_JUPYTER` or `QtCore/QtGui/ QtWidgets` in `fastplotlib.utils.gui`, through a module `__getattr__`. `fpl.loop` and `fastplotlib.utils.loop` are lazy in the same way. In practice the selection now happens when a Figure is created with `canvas=None`, or when `fpl.loop` is used. Resume this Claude session: ``` cd /home/mark/git/pygfx/fastplotlib claude --resume 079dc15c-fc70-4763-9f9b-515f8af4b56e ``` </details>
| @@ -1,5 +1,4 @@ | |||
| # this MUST be imported as early as possible in fpl.__init__ before any other wgpu stuff | |||
| from .gui import loop | |||
| from . import gui | |||
There was a problem hiding this comment.
Having this is a little "hostile" to people using different loops and event managers.
I'm hoping you can delay this?
|
@kushalkolar before really going into the details of this change request, i'm hoping we can relax starting the loop as early as possible. It seems a bit of an overreach when we think of fastplotlib as a "library" and not an application. The choice of the loop is really an "application level decision". LMK what you think |
|
I think I like this overall. @almarklein @clewis7 what do you think? I'm leaning on removing the banner with available GPUs list, is anyone using even notebooks and looking at these things anymore these days 🤷 . It was inspired by the bokeh banner |
|
I saw this change list and it was a little large. Thx for considering alternatives. |
Is this you Mark or Claude 😂 |
|
Btw @hmaarrfk we should meet again when you're in NYC! |
|
I agree that fpl is a library and it would be good to allow the application code to have more control over loops etc. I think this PR tries to achieve that without breaking the API. But there's a lot of module-level The 'cleanest' solution is probably to do what PyGfx does and let the user create the canvas and loop. But FPL tries to abstract some of that away, so not sure. |
i mostly exclusively code in claude now. In truth upon reviewing the changelist, I totally understood what it was trying to acheive, that is what almar said "not changing the API", and in python land everything is part of the API, so the use of If you are ok with changing a bit of the API, i think we can "avoid importing loop", until we absolutely need it in the functions that need it. And this would avoid the whole |
|
Looks like I forgot to put examples/diffs in .gitignore |
ff19d86 to
50a7a1b
Compare
<details><summary>Claude's draft</summary> Following the review discussion, replace the lazy module `__getattr__` approach with a simpler one: fastplotlib no longer makes any application-level GUI decision at import time. - Remove `fastplotlib/utils/gui.py`. With it go the Qt priming, the Jupyter adapter banner, and the `GUI_BACKEND`, `IS_JUPYTER` and `QtCore/QtGui/QtWidgets` re-exports, none of which were used internally. - `make_canvas_and_renderer` and the NDWidget fetch scheduler import `rendercanvas.auto` inside the function, only when they need it. `BaseRenderCanvas` comes straight from `rendercanvas`. - `fpl.loop` and `fastplotlib.utils.loop` are removed, with no deprecation shim. - Examples and the user guide use `from rendercanvas.auto import loop`. Resume this Claude session: ``` cd /home/mark/git/pygfx/fastplotlib claude --resume ae74f971-2c13-46b0-a892-ebcf861a72ec ``` </details>
50a7a1b to
235313c
Compare
I figured that was the case, can you confirm that you were describing in fact just riping out this whole loop stuff |
I think it makes sense if people are using different loops as you said. I haven't encountered this though, curious where you've run into it. |
|
i guess i kinda said it right? We are building our application, and thus want tighter control of the event loop, so having things depend on import order is a little strange and frustrating to nail down. We are evaluating different loops, and different optimizations between them, so like having this parameter to dance around is a little annoying, and we often work "offscreen". |
yea I think that makes sense! |
|
ok marked as ready for review. thx |
|
@FlynnOConnell @apasarkar once this is merged you'll need to import |
|
Seems like |
We can temporarily add a small module-level |
|
Please feel free to push directly here, i'll have my AI not squash your changes. |
@kushalkolar It is helpful for some notebook users, it's been used in the last few weeks as a quick "hey this isn't right" |
|
I'm on the fence about removing the notebook banner. |
|
Can we just print the parrot and selected device? Not all avail |
Remove the loop selection code from fastplotlib allowing downstream applications to make the choice.
Claude
Motivation
fastplotlib is often used as a library inside a larger application, not as the application itself. That application decides which windowing system and event loop to use, or runs headless/offscreen and passes its own
canvasandrenderertoFigure.Before this PR,
import fastplotlibranfrom rendercanvas.auto import RenderCanvas, loopat import time, throughfastplotlib/utils/gui.py. It also tried to import a Qt library first, so that Qt would be preferred. As a result:rendercanvas.autoraisesImportErrorat module level).RENDERCANVAS_BACKENDbefore the first import, which leaks a process-wide setting into the host application.How we got here
fpl.loop,fastplotlib.utils.gui.GUI_BACKEND,IS_JUPYTER,QtWidgets, ...) and made the backend selection lazy through module-level__getattr__in three modules.__getattr__magic, and that the cleanest model is PyGfx's: the user creates the canvas and loop. @kushalkolar was in favor of removing the Jupyter GPU-adapter banner. The maintainers then indicated they are fine with a more drastic, API-changing version.rendercanvas.autois imported only inside the functions that need it, and fastplotlib no longer makes GUI decisions on import.Final state
fastplotlib/utils/gui.pyis removed. With it go:rendercanvas.autodecides, like in PyGfx);GUI_BACKEND,IS_JUPYTERandQtCore/QtGui/QtWidgetsre-exports. None of them were used inside fastplotlib.make_canvas_and_rendererimportsrendercanvas.auto.RenderCanvasonly whenFiguregetscanvas=None.BaseRenderCanvasis imported directly fromrendercanvas.widgets/nd_widget/_index.py) importsrendercanvas.auto.loopinside_schedule_fetch.fpl.loopandfastplotlib.utils.loopare removed, with no deprecation shim, so there is no module__getattr__left anywhere. Scripts that callfpl.loop.run()now get anAttributeErrorand should switch torendercanvas.auto.loop.Behavior changes to review
rendercanvas.autotries glfw before Qt. Users who have both installed and relied on fastplotlib preferring Qt now get glfw. They can import their Qt lib first or setRENDERCANVAS_BACKEND=qt.fpl.enumerate_adapters()/fpl.print_wgpu_report()remain for checking GPUs.fpl.loop,fastplotlib.utils.loop,fastplotlib.utils.gui(and everything in it).Verification
import fastplotlibsucceeds without importingrendercanvas.autoor any GUI lib.Figure(canvas=OffscreenRenderCanvas(...), renderer=WgpuRenderer(...))andFigure(canvas="offscreen")show and draw, still without importingrendercanvas.auto.Figure()raises rendercanvas'ImportError, as expected.rendercanvas.autoafterimport fastplotlib.fpl.loopno longer exists.fpl.Figure()creates aQRenderCanvas.RENDERCANVAS_FORCE_OFFSCREEN=1 pytest tests(withouttests/test_recording.py, which fails locally on a missing PyAV codec, same as onmain): 1548 passed.RENDERCANVAS_FORCE_OFFSCREEN=1 pytest examples: 195 passed, 21 failed. The same 21 fail before this change (missing optional deps such assklearnin my env).Resume this Claude session: