feat - Add tight_layout support to reduce margins and improve layout spacing(new)#1182
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdds an optional tight_layout flag to Visdom Plotly layouts that reduces default margins and enables Plotly’s automargin behavior on 2D axes while preserving existing explicit margin overrides and 3D layout behavior. Flow diagram for tight_layout handling in _opts2layoutflowchart TD
A[opts input] --> B[Read tight_layout from opts]
B --> C{is3d?}
C -- Yes --> D[Use 3D defaults:
margin.l = opts.marginleft or 0
margin.b = opts.marginbottom or 0
margin.t = opts.margintop or 20
margin.r = opts.marginright or 60]
C -- No --> E{tight_layout?}
E -- No --> F[Use 2D defaults:
margin.l = opts.marginleft or 60
margin.b = opts.marginbottom or 60
margin.t = opts.margintop or 60
margin.r = opts.marginright or 60]
E -- Yes --> G[Use tight 2D defaults:
margin.l = opts.marginleft or 0
margin.b = opts.marginbottom or 0
margin.t = opts.margintop or 30
margin.r = opts.marginright or 0]
D --> H[Build layout dict]
F --> H
G --> H
H --> I[axis automargin = opts.tight_layout or None in _axisformat]
I --> J[Return final layout and axes]
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
tight_layout, theenvparameter defaults toNoneand is passed through directly toget_window_data/_update_window_tight_layout; consider normalizingenvtoenv or self.envfor consistency with the rest of the API. _update_window_tight_layoutcurrently overwrites all margins with zeros regardless of any existing explicit margin configuration ortight_layout-related logic in_opts2layout; consider reusing_opts2layoutor conditionally preserving user-specified margins to avoid surprising layout changes.- When calling
self._sendin_update_window_tight_layout, you setoptsto{"tight_layout": True}which discards any existingoptsassociated with the window; consider merging this flag into the existing opts instead of replacing them.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `tight_layout`, the `env` parameter defaults to `None` and is passed through directly to `get_window_data`/`_update_window_tight_layout`; consider normalizing `env` to `env or self.env` for consistency with the rest of the API.
- `_update_window_tight_layout` currently overwrites all margins with zeros regardless of any existing explicit margin configuration or `tight_layout`-related logic in `_opts2layout`; consider reusing `_opts2layout` or conditionally preserving user-specified margins to avoid surprising layout changes.
- When calling `self._send` in `_update_window_tight_layout`, you set `opts` to `{"tight_layout": True}` which discards any existing `opts` associated with the window; consider merging this flag into the existing opts instead of replacing them.
## Individual Comments
### Comment 1
<location path="py/visdom/__init__.py" line_range="267-272" />
<code_context>
+ layout["xaxis"] = {}
+ if not layout.get("yaxis"):
+ layout["yaxis"] = {}
+ layout["xaxis"]["automargin"] = True
+
if opts.get("stacked"):
</code_context>
<issue_to_address>
**suggestion:** Consider enabling `automargin` on both x and y axes for tight 2D layouts.
In the `tight and not is3d` branch, only `xaxis.automargin` is set while `yaxis` is left unchanged. For tight layouts this can cause asymmetric behavior and potential clipping on the vertical side. Please apply the same `automargin` logic to `yaxis` (including initializing the dict if needed) so both axes behave consistently.
```suggestion
if tight and not is3d:
if not layout.get("xaxis"):
layout["xaxis"] = {}
if not layout.get("yaxis"):
layout["yaxis"] = {}
layout["xaxis"]["automargin"] = True
layout["yaxis"]["automargin"] = True
```
</issue_to_address>
### Comment 2
<location path="py/visdom/__init__.py" line_range="1646-1650" />
<code_context>
+ if raw is None:
+ return
+ try:
+ win_map = json.loads(raw) if isinstance(raw, str) else raw
+ except (ValueError, TypeError):
+ return
+ results = {}
+ for win_id, win_info in win_map.items():
+ if not isinstance(win_info, dict) or win_info.get("type") != "plot":
+ continue
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard against non-dict `win_map` before iterating with `.items()` in `tight_layout`.
After decoding `raw`, this code assumes `win_map` is a mapping and calls `.items()` unconditionally. If the backend returns something else (e.g., a list or `None`), this will raise an `AttributeError`. Please add a type check (e.g., `isinstance(win_map, dict)`) and return early if it fails to ensure robustness against unexpected responses.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The runtime
tight_layoutupdate sets all margins to 0 (including top) whereas_opts2layoutkeeps a 30px top margin when a title exists, which may lead to inconsistent behavior and potential title clipping between newly created plots and retrofitted ones. - In
_update_window_tight_layout, you send"opts": {"tight_layout": True}on update, which will discard any existingoptsfor that window; consider merging with existing options instead of overwriting them to avoid losing configuration.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The runtime `tight_layout` update sets all margins to 0 (including top) whereas `_opts2layout` keeps a 30px top margin when a title exists, which may lead to inconsistent behavior and potential title clipping between newly created plots and retrofitted ones.
- In `_update_window_tight_layout`, you send `"opts": {"tight_layout": True}` on update, which will discard any existing `opts` for that window; consider merging with existing options instead of overwriting them to avoid losing configuration.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@marcoag Can u Please review this PR ? |
|
Hi @Manik-Khajuria-5, I triggered a copilot review - please address its concerns. |
There was a problem hiding this comment.
Pull request overview
Adds a tight_layout feature to Visdom plot layouts to reduce default Plotly margins and enable axis automargin, plus a client API (viz.tight_layout) to apply the behavior to existing windows.
Changes:
- Extend
_opts2layoutto supportopts["tight_layout"]=Truewith reduced default margins and axisautomargin(2D). - Add
_update_window_tight_layout(...)to update an existing window’s Plotly layout in-place. - Add public
tight_layout(win=None, env=None)to apply tight layout to one window or all plot windows in an env.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Jayantparashar10
left a comment
There was a problem hiding this comment.
The _opts2layout change handles tight_layout=True correctly t: 30 when a title exists, t: 0 otherwise. Good.
But viz.tight_layout(win=...) goes through a completely different path _update_window_tight_layout fetches the current window layout and manually overwrites the margin:
margin.update({"l": 0, "r": 0, "b": 0, "t": 0})
This always sets t: 0 regardless of whether the window has a title.
Tested locally — calling viz.tight_layout(win=w) on a titled plot clips the title entirely.
The fix is straightforward since you already fetched the layout:
has_title = bool(layout.get("title"))
margin.update({"l": 0, "r": 0, "b": 0, "t": 30 if has_title else 0})
|
@Jayantparashar10 Done Thanks for review! |
Jayantparashar10
left a comment
There was a problem hiding this comment.
tight_layout() is missing @pytorch_wrap without it the method won't be recorded in offline mode, unlike every other public Visdom method. Add @pytorch_wrap decorator above the method definition to match the convention.
|
@Jayantparashar10 Thanks for review I checked in codebase methods like scatter , line , image need it but tight_layout is closer to utility methods like save, close, delete_env, and more So it is not needed |
|
@rajnisht7 Thanks for review . I have fixed it kindly review it again |
There was a problem hiding this comment.
@Manik-Khajuria-5 when resizing the pane to minimum width, visdom is crashing
also still the title is overlapping with the mod bar
Screencast_20260530_102303.online-video-cutter.com.mp4
|
@rajnisht7 Thanks for reveiw I have fixed recommended suggestion |
e6d3ae8 to
441b217
Compare
|
Hey @rajnisht7, you were right Plotly already has built-in tight layout support via automargin on axes. Thanks for pointing this out in the last meeting! I've |
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Consider validating or normalizing
opts['tight_layout']to a strict boolean before using it (rather than relying on general truthiness), so unexpected types (e.g., strings from JSON) don’t silently trigger tight layout andautomargin. - The tightened margin defaults are now embedded directly in
_opts2layout; if other plot types or future layout options need similar behavior, it might be worth centralizing these margin presets (normal vs tight vs 3D) into named constants or a small helper for easier tuning.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider validating or normalizing `opts['tight_layout']` to a strict boolean before using it (rather than relying on general truthiness), so unexpected types (e.g., strings from JSON) don’t silently trigger tight layout and `automargin`.
- The tightened margin defaults are now embedded directly in `_opts2layout`; if other plot types or future layout options need similar behavior, it might be worth centralizing these margin presets (normal vs tight vs 3D) into named constants or a small helper for easier tuning.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@rajnisht7 Can u please review this PR too |






Description
This PR adds a
tight_layoutoption to Visdom plots, leveraging Plotly's built-inautomargininstead of hardcoding margins.Changes:
_opts2layoutupdates:tight_layout=Truein optsmarginleft, etc.) still override defaults_axisformatupdates:automarginon 2D axes whentight_layout=TrueMotivation and Context
Visdom plots often have excessive default margins (60px on all sides), reducing usable space.
As noted in #724, both matplotlib (
tight_layout()) and Plotly (automargin) handle this natively.This feature enables Plotly's built-in layout intelligence via a simple opt-in flag no hardcoded zero margins or extra API methods needed.
Fixes: #724
How Has This Been Tested?
test_tight_layout.pywith side-by-side comparison of tight vs normal plotstight_layout=Truetight_layout=TrueScreenshots (if appropriate):
Types of changes
Checklist:
py/visdom/VERSIONaccording to Semantic VersioningSummary by Sourcery
Add a tight_layout option to Visdom plots to reduce default margins and leverage Plotly’s automatic margin handling.
New Features:
Enhancements: