Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make default notebook widget size smaller and configurable #6827

Merged
merged 5 commits into from
Jul 9, 2024

Conversation

jprochazk
Copy link
Member

@jprochazk jprochazk commented Jul 9, 2024

What

Checklist

  • I have read and agree to Contributor Guide and the Code of Conduct
  • I've included a screenshot or gif (if applicable)
  • I have tested the web demo (if applicable):
  • The PR title and labels are set such as to maximize their usefulness for the next release's CHANGELOG
  • If applicable, add a new check to the release checklist!

To run all checks from main, comment on the PR with @rerun-bot full-check.

@jprochazk jprochazk added notebook Jupyter notebooks etc include in changelog labels Jul 9, 2024
@jprochazk jprochazk added exclude from changelog PRs with this won't show up in CHANGELOG.md and removed notebook Jupyter notebooks etc include in changelog labels Jul 9, 2024
@emilk emilk changed the title Update default notebook widget size Make default notebook widget size smaller (640x480) Jul 9, 2024
@Wumpf
Copy link
Member

Wumpf commented Jul 9, 2024

not sure if that's what we want? The original ticket called for hyper intelligent (😉) sizing where we do stuff like detecting that an incoming image has a certain size.
No idea what a good size is for a widget in a notebook, why is this one better than the previous?

@jprochazk
Copy link
Member Author

jprochazk commented Jul 9, 2024

I misunderstood. This is just the first part of the issue, then. It's a better default size because it's smaller, taking up less vertical space, and with a 6.4 / 4.8 aspect ratio. The original issue asked for 6.4 x 4.8 inches, but I don't think we actually want to use inches as a unit, even though it's possible.

We can dynamically size the viewer as well, by updating its style. We can do that at any level (in viewer, in web-viewer JS, in widget JS, in notebook.py), but I'm not sure if we actually want to do that. 🤔

@jleibs
Copy link
Member

jleibs commented Jul 9, 2024

It could be nice if the defaults were something the user could set once as a global instead of needing to override on every call you notebook_show.

@jprochazk
Copy link
Member Author

It could be nice if the defaults were something the user could set once as a global instead of needing to override on every call you notebook_show.

What would that look like? I don't know what would be idiomatic in Python, at least the following would be possible:

# top-level function
from rerun.notebook import set_default_size
set_default_size(width=640, height=480)
# static method
from rerun.notebook import Viewer
Viewer.set_default_size(width=640, height=480)
# "static" property
from rerun.notebook import Viewer
Viewer.size = (640, 840)

@jleibs
Copy link
Member

jleibs commented Jul 9, 2024

I don't know what would be idiomatic in Python, at least the following would be possible:

@abey79 do you have any thoughts on this?

I find the top-level function design:

from rerun.notebook import set_default_size
set_default_size(width=640, height=480)

to be the most straight-forward to think about.

@abey79
Copy link
Member

abey79 commented Jul 9, 2024

I find the top-level function design to be the most straight-forward to think about.

Agreed.

@jprochazk jprochazk changed the title Make default notebook widget size smaller (640x480) Make default notebook widget size smaller and configurable Jul 9, 2024
@jprochazk
Copy link
Member Author

Alright, I added rerun.notebook.set_default_size

@jprochazk jprochazk merged commit 6b2671d into main Jul 9, 2024
18 of 19 checks passed
@jprochazk jprochazk deleted the jan/default-notebook-size branch July 9, 2024 17:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exclude from changelog PRs with this won't show up in CHANGELOG.md
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants