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

🌅 FigureResampler display improvements #97

Merged
merged 5 commits into from
Jul 11, 2022

Conversation

jvdd
Copy link
Member

@jvdd jvdd commented Jul 8, 2022

This PR does;

  • remove the empty space under inline FigureResampler when height is not passed
    (this is because the default plotly figure height is 450, but the default jupyter-dash app height is 650 👉 👈 )
  • add show_dash_kwargs to FigureResampler
  • test the above
  • when ipython display is called on a FigureResampler it will start an inline jupyter-dash app
  • extend tests to also check for other properties that are not in _get_pr_props_keys

This PR adds the following behavior to FigureResampler; when having fig as output in your notebook (_ipython_display_) -> now a dash app will be started under the hood with mode = "inline"

Users can control the port and other properties of the dash app via the show_dash_kwargs that is added to the constructor of FigureResampler. This can be set in two manners;

  • fig = FigureResampler(..., show_dash_kwargs=dict(port=8051))
  • register_plotly_resampler(mode="figure", show_dash_kwargs=dict(port=8051))

@jvdd jvdd requested a review from jonasvdd July 8, 2022 16:06
@jvdd
Copy link
Member Author

jvdd commented Jul 9, 2022

@jonasvdd this PR should only be merged if you are fine with adding the behavior described above (of course)!

@jonasvdd
Copy link
Member

@jvdd, I like this behavior!

@jonasvdd jonasvdd self-assigned this Jul 11, 2022
@jonasvdd jonasvdd added the enhancement New feature or request label Jul 11, 2022
@jonasvdd
Copy link
Member

I think this is somewhat related to the issues in #99, maybe we can improve the documentation about the dynamic aggregation as-well?

@jvdd
Copy link
Member Author

jvdd commented Jul 11, 2022

Jup @jonasvdd,

I think revisiting the documentation will never hurt, certainly with the many new PRs that recently have been merged!

@@ -41,6 +41,7 @@ def __init__(
show_mean_aggregation_size: bool = True,
convert_traces_kwargs: dict | None = None,
verbose: bool = False,
show_dash_kwargs: dict | None = None,
Copy link
Member

Choose a reason for hiding this comment

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

there is no (generated) documentation for this arg as the documentation from the superclass its __init__ is used

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants