Skip to content

feat: more complete main window implementation - #741

Open
larsoner wants to merge 53 commits into
pyapp-kit:mainfrom
larsoner:main-window-604
Open

feat: more complete main window implementation#741
larsoner wants to merge 53 commits into
pyapp-kit:mainfrom
larsoner:main-window-604

Conversation

@larsoner

Copy link
Copy Markdown

Closes #601
Closes #604

So... the "next week" I promised in 2023 has finally arrived 😅 This PR has the 43 commits from that PR, plus what Claude Fable found while reviewing (below is its summary). I reviewed the commits and they make sense to me. Notably I removed the backend autodetection since we should probably scope / implement that separately. (Automatic IPython importing can be problematic, and maybe it should be opt-in, etc.)

Before the Fable fixes:

mw_shot

After:

mw_shot4

Qt (I'm on macOS so the menu isn't captured in-window):

pr604_qt
  • Merged with current main (~3 yrs of drift)
  • Made the ipynb MainWindow work as a real Container: children now flow into the grid's central cell (previously main.append(...) raised KeyError).
  • Implemented ipynb MenuBar/Menu as a row of Dropdowns per the MainWindow for ipywidgets #601 discussion (first entry is the title placeholder; selecting an entry fires its callback and resets); nested submenus raise NotImplementedError for now.
  • Fixed grid placement: each bar/dock box gets its own Layout instance (they shared one, so GridspecLayout.__setitem__ stacked all eight into a single cell), and the grid templates are set via the attributes _update_layout() re-applies, so empty areas collapse and the central widget gets 1fr.
  • ipynb StatusBar: added widgets are tracked separately from the message label and spacer, negative insert positions append (matching Qt), and message timeouts use get_running_loop() instead of the deprecated get_event_loop().
  • set_menu_bar(None) / set_status_bar(None) no longer crash in the ipynb backend.
  • Drive-by fix for a pre-existing bug on main: Qt Container/Dialog reported layout == "vertical" regardless of orientation (isinstance(self, QHBoxLayout) can never be true for a backend widget).
  • Exported Menu/MenuBar from magicgui.widgets, and fixed a raise self._ipywidgetreturn typo in _mgui_get_native_layout.
  • Added backend-parametrized tests for the dock/toolbar/status-bar/menu APIs (should address the codecov failure).
  • Moved the root example.py into docs/examples/applications/main_window.py.
  • Dropped the _choose_backend() jupyter auto-detection for now — it's separable and changes default-backend behavior for all users, so it can be its own PR.

tlambert03 and others added 22 commits October 20, 2023 13:33
Resolves conflicts from ~3 years of drift. Notable resolution: keep the
Container-based Qt MainWindow composition from pyapp-kit#686 (central widget
swapped under a QMainWindow) and graft on the new
_mgui_add_dock_widget/_mgui_add_tool_bar/_mgui_set_status_bar/
_mgui_set_menu_bar methods, instead of this branch's QBaseWidget
rewrite that reimplemented the container protocol by hand.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…f layout

isinstance(self, QtW.QHBoxLayout) can never be true for a backend
widget; check the layout object instead.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
- MainWindow now builds on Container: the H/VBox created by Container
  becomes the central widget of the IpyMainWindow grid, and container
  children flow into it (previously append() raised KeyError because
  __init__ skipped Container entirely and nothing was wired to the
  central cell).  Container child management is routed through a _box
  attribute so subclasses can redirect it.
- Grid uses auto/1fr templates so empty bars collapse and the central
  widget takes the remaining space (per the mapping discussed in pyapp-kit#601).
- MenuBar/Menu implemented as a row of Dropdowns: the first entry acts
  as the menu title, selecting an entry fires its callback and resets
  the selection.  Nested menus raise NotImplementedError for now.
- StatusBar: added widgets tracked separately from the message label and
  spacer; negative insert positions append (matching Qt, where
  insertWidget(-1) is out-of-range); use get_running_loop() for message
  timeouts instead of the deprecated get_event_loop().
- set_menu_bar(None)/set_status_bar(None) no longer raise.
- Fix _mgui_get_native_layout (raise -> return) and export Menu/MenuBar
  from magicgui.widgets.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ends

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Auto-selecting the ipynb backend inside a Jupyter kernel is separable
from the main-window work and changes behavior for all existing
notebook users of the qt backend, so it should be its own PR.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GridspecLayout.__setitem__ writes the cell position into
child.layout.grid_area, so the bar/dock boxes must not share a Layout
instance (previously all eight ended up stacked in one cell).  Also set
the grid templates via the private attributes that
GridspecLayout._update_layout re-applies on every assignment, so empty
bars collapse and the central widget row/column gets 1fr.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Matches Qt, where a toolbar spans its area and add_spacer() pushes
subsequent items to the far side.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 44 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.97%. Comparing base (2f08fb3) to head (846a9c8).

Files with missing lines Patch % Lines
src/magicgui/backends/_ipynb/widgets.py 85.14% 26 Missing ⚠️
src/magicgui/backends/_qtpy/widgets.py 86.53% 14 Missing ⚠️
src/magicgui/widgets/bases/_menubar.py 93.10% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #741      +/-   ##
==========================================
+ Coverage   89.32%   89.97%   +0.64%     
==========================================
  Files          40       43       +3     
  Lines        4835     5245     +410     
==========================================
+ Hits         4319     4719     +400     
- Misses        516      526      +10     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

test_widgets.py::test_bound_callable_catches_recursion carries a FIXME
saying it must run before any test swaps backends between qt and ipynb,
or it stack-overflows on Windows.  test_main_window.py sorted before
test_widgets.py and its backend-parametrized tests swap backends, which
crashed the three Windows jobs on CI.  Rename the file so it collects
after test_widgets.py.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
larsoner added a commit to larsoner/magicgui that referenced this pull request Aug 24, 2026
The namespace from runpy.run_path is discarded, so widgets created by
an example are garbage-collected at an arbitrary later point during
event processing.  A still-visible widget whose C++ side is deleted
mid-event-loop can receive a paintEvent during deletion, which PyQt5
escalates to a process abort:

    RuntimeError: wrapped C/C++ object of type QRangeSlider has been
    deleted

as seen intermittently on the windows-latest (3.12) pyqt5 job in
test_example[demo_widgets/range_slider.py] (e.g. the scheduled main
run on 2026-08-22, and both pyapp-kit#741 and pyapp-kit#742).

Close (hide) all top-level widgets first so nothing can paint, then
deleteLater() + sendPostedEvents(None, DeferredDelete) to destroy the
native widgets (and stop their timers) while hidden -- note that
processEvents() alone does NOT dispatch DeferredDelete events -- then
gc.collect() for anything left over.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.

MainWindow for ipywidgets

2 participants