Skip to content

Support initial selection in Plotly plots #6747

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Oct 13, 2025
Merged

Support initial selection in Plotly plots #6747

merged 2 commits into from
Oct 13, 2025

Conversation

@yairchu
Copy link
Contributor

@yairchu yairchu commented Oct 12, 2025

📝 Summary

Fixes #6746 for Plotly (Altair issue unchanged)

🔍 Description of Changes

mo.ui.plotly supports plots having an initial selection made by Plotly'a add_selection method.

📋 Checklist

  • I have read the contributor guidelines.
  • I have added tests for the changes made.
    • Didn't find a plotly equivalent for tests/_plugins/ui/_impl/test_altair_chart.py. If there is one I'll gladly add a test
  • I have run the code and verified that it works as expected.

@vercel
Copy link

vercel bot commented Oct 12, 2025 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
marimo-docs Ready Ready Preview Comment Oct 12, 2025 7:36pm

@yairchu yairchu force-pushed the plotly-init-select branch from b8f494f to 07740e4 Compare October 12, 2025 15:28
akshayka
akshayka reviewed Oct 12, 2025
View reviewed changes
Copy link
Contributor

@akshayka akshayka left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the enthusiasm! However, before filing PRs, we request that contributors align with us in the issue on whether we're receptive to the PR, as well as the approach.

In this case, your PR adds a new public API, which requires serious consideration and discussion from the team, because new APIs add complexity and maintenance burden.

Instead of discussing these topics at the code stage, we request our contributors to discuss with us when filing an issue. You should have seen a sentence to this effect in the issue template; sorry if it wasn't sufficiently clear.

@yairchu
Copy link
Contributor Author

yairchu commented Oct 12, 2025

In this case, your PR adds a new public API, which requires serious consideration and discussion from the team, because new APIs add complexity and maintenance burden.

I'm not sure what you mean. Which new public API is this PR adding?

@akshayka
Copy link
Contributor

akshayka commented Oct 12, 2025 •
edited
Loading

In this case, your PR adds a new public API, which requires serious consideration and discussion from the team, because new APIs add complexity and maintenance burden.

I'm not sure what you mean. Which new public API is this PR adding?

add_selection

Sorry, misread the PR reading it quickly!

@yairchu yairchu force-pushed the plotly-init-select branch from 07740e4 to e2d1d40 Compare October 12, 2025 19:11
@yairchu yairchu changed the title Plotly init select Support initial selection in Plotly plots Oct 12, 2025
@yairchu
Support initial range selections in plotly plots
368d59d 
Does not initialize the selected items!
@yairchu
Popuplate initial plotly values selections
3cd5b99 
Having not found a more elegant solution,
we iterate over the figure's data to create the initial values in the marimo wrapper side.
@yairchu yairchu force-pushed the plotly-init-select branch from e2d1d40 to 3cd5b99 Compare October 12, 2025 19:35
@mscolnick mscolnick added the bug Something isn't working label Oct 13, 2025
@mscolnick
Copy link
Contributor

Didn't find a plotly equivalent for tests/_plugins/ui/_impl/test_altair_chart.py. If there is one I'll gladly add a test

@yairchu we likely are missing tests for tests/_plugins/ui/_impl/test_altair_chart.py. i can followup with some. thanks for the fix!

@mscolnick mscolnick merged commit 23900ec into marimo-team:main Oct 13, 2025
36 of 37 checks passed
@mscolnick mscolnick mentioned this pull request Oct 13, 2025
Merged
@yairchu yairchu deleted the plotly-init-select branch October 13, 2025 21:42
mscolnick added a commit that referenced this pull request Oct 14, 2025
@mscolnick
tests: mo.ui.plotly tests (#6757)
b1228b4 
Follow tests for #6747

cc @yairchu
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@akshayka akshayka akshayka left review comments

@mscolnick mscolnick mscolnick approved these changes

Assignees

No one assigned

Labels

bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support plots with initial selection

3 participants

@yairchu @akshayka @mscolnick