FIX colorbar placement when passing axis to quickshow#1
Conversation
When an axis (subplot) is passed to cortex.quickshow, the colorbar was being placed using figure-relative coordinates, causing it to appear outside the subplot bounds. This fix: - Adds a helper function _transform_colorbar_location() to convert colorbar coordinates from axis-relative to figure-relative - Updates add_colorbar() and add_colorbar_2d() to accept ax_bounds parameter for coordinate transformation - Updates make_figure() to detect when a user-provided axis is used and pass its bounds to the colorbar functions
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where colorbars were incorrectly placed outside subplot bounds when a user provides their own axis to cortex.quickshow(). The issue occurred because colorbar coordinates were being interpreted as figure-relative rather than axis-relative.
Key changes:
- Added coordinate transformation helper function to convert axis-relative to figure-relative coordinates
- Updated colorbar functions to accept optional axis bounds parameter
- Modified
make_figure()to detect user-provided axes and pass bounds for proper colorbar placement
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| cortex/quickflat/view.py | Added ax_bounds tracking to detect when user provides an axis, and passes bounds to colorbar functions for correct placement |
| cortex/quickflat/composite.py | Added _transform_colorbar_location() helper and updated add_colorbar() and add_colorbar_2d() to accept ax_bounds parameter for coordinate transformation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| cimg : matplotlib.image.AxesImage object | ||
| Image for which to create colorbar. For reference, matplotlib.image.AxesImage | ||
| Image for which to create colorbar. For reference, matplotlib.image.AxesImage | ||
| is the output of imshow() |
There was a problem hiding this comment.
The parameter documentation is incorrect. The function signature has cmap_name and colorbar_ticks as parameters, not cimg. The description should match the actual parameter cmap_name which is the name of the colormap to use.
| colorbar_location : array-like | ||
| Four-long list, tuple, or array that specifies location for colorbar axes | ||
| Four-long list, tuple, or array that specifies location for colorbar axes | ||
| [left, top, width, height] (?) |
There was a problem hiding this comment.
The documentation says "[left, top, width, height]" but the implementation expects "[left, bottom, width, height]" (note: bottom, not top). This should be corrected to accurately reflect the coordinate system being used. The second value represents the bottom position in normalized figure coordinates.
| colorbar_location : array-like | ||
| Four-long list, tuple, or array that specifies location for colorbar axes | ||
| Four-long list, tuple, or array that specifies location for colorbar axes | ||
| [left, top, width, height] (?) |
There was a problem hiding this comment.
The documentation says "[left, top, width, height]" but the implementation expects "[left, bottom, width, height]" (note: bottom, not top). This should be corrected to accurately reflect the coordinate system being used. The second value represents the bottom position in normalized figure coordinates.
| # Get axis bounds for colorbar placement within subplot | ||
| ax_bounds = ax.get_position().bounds |
There was a problem hiding this comment.
The new colorbar placement functionality when passing an axis to quickshow lacks test coverage. Consider adding a test that verifies the colorbar is placed correctly within subplot bounds when a user-provided axis is used, similar to the existing test_colorbar_location but with a figure containing multiple subplots.
- Fix docstring in add_colorbar_2d: replace incorrect 'cimg' parameter with actual 'cmap_name' parameter and add missing 'fontsize' docs - Fix coordinate documentation in add_colorbar and add_colorbar_2d: change '[left, top, width, height]' to '[left, bottom, width, height]' - Add test_colorbar_placement_with_subplot to verify colorbars are placed within subplot bounds when an axis is passed to quickshow
Address all issues from code review: Production fixes (cortex/webgl/jupyter.py): - Use 127.0.0.1 for IFrame URL instead of socket.gethostname() (#1) - Add CORTEX_JUPYTER_IFRAME_HOST env var for remote setups - Pass display_url=False to suppress redundant URL output (gallantlab#2) - Release _viewer_lock before blocking close() in close_all() (gallantlab#4) - Clean up HTTP server and tmpdir if ipydisplay() fails (gallantlab#7) - Clean up tmpdir if HTTPServer() constructor fails (gallantlab#8) - Guard _repr_html_() against use-after-close (gallantlab#9) - Add public `closed` property (gallantlab#10) - Add context manager protocol (__enter__/__exit__) (gallantlab#11) - Warn when server thread doesn't terminate after join (gallantlab#14) - Remove viewer from _active_viewers on close - Harden close_all() for interpreter shutdown Config fixes: - Guard nbsphinx import in docs/conf.py (gallantlab#5) - Change nbsphinx_execute to "auto" (gallantlab#6) - Protect jupyter import in __init__.py (gallantlab#16) - Remove dead notebook.html template (gallantlab#3) Tests (21 new, 1 fixed): - IFrame URL correctness and env var configurability - display_url=False verification - closed property, _repr_html_ after close - Context manager lifecycle - Double-close idempotency - Graceful degradation when httpd.shutdown() fails - Thread join timeout warning - close_all() lock scope (deadlock prevention) - Resource leak on ipydisplay/HTTPServer failure - FileNotFoundError when index.html missing - CORTEX_JUPYTER_STATIC_HOST env var - HTTP server integration test (serves content, logs 404s) - make_notebook_html kwargs forwarding - Registry removal on close - Replace flaky port uniqueness test (gallantlab#24) Docs notebook: - Add disk-write warning for static viewer - Rewrite to be executable during docs build
When an axis (subplot) is passed to cortex.quickshow, the colorbar
was being placed using figure-relative coordinates, causing it to
appear outside the subplot bounds.
This fix:
colorbar coordinates from axis-relative to figure-relative
parameter for coordinate transformation
and pass its bounds to the colorbar functions