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

go.Figure.set_subplots #2866

Merged
merged 5 commits into from
Nov 5, 2020
Merged

go.Figure.set_subplots #2866

merged 5 commits into from
Nov 5, 2020

Conversation

nicholas-esterer
Copy link
Contributor

@nicholas-esterer nicholas-esterer commented Oct 30, 2020

Closes #2405

This is now possible

import plotly.graph_objects as go
figgy=go.Figure().set_subplots(2,3)

and figgy is equivalent to what is given by:

from plotly.subplots import make_subplots
figgy=make_subplots(2,3)

Code PR

  • [ x] I have read through the contributing notes and understand the structure of the package. In particular, if my PR modifies code of plotly.graph_objects, my modifications concern the codegen files and not generated files.
  • [ x] I have added tests (if submitting a new feature or correcting a bug) or
    modified existing tests.
  • For a new feature, I have added documentation examples in an existing or
    new tutorial notebook (please see the doc checklist as well).
  • I have added a CHANGELOG entry if fixing/changing/adding anything substantial.

Calling `fig.set_subplots(2,3)` works just like fig=make_subplots(2,3).
Also accepts the same keywords arguments as make_subplots. Fails if fig
already has subplots.

Tests still need to be added.
@nicholas-esterer nicholas-esterer requested review from nicolaskruchten and emmanuelle and removed request for nicolaskruchten November 2, 2020 20:44
@nicolaskruchten nicolaskruchten changed the title go.Figure.set_sublots go.Figure.set_subplots Nov 2, 2020
@nicolaskruchten
Copy link
Contributor

@jonmmease mind taking a quick look here to see if everything makes sense?

Copy link
Contributor

@jonmmease jonmmease left a comment

Choose a reason for hiding this comment

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

This looks like a nice addition! One small suggestion on the tests, but otherwise I'm happy with it.

assert fig1.layout == fig1_sp.layout
# Test that calling on a figure that already has subplots throws an error.
raised = False
try:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd recommend using with pytest.raises (https://docs.pytest.org/en/stable/assert.html#assertions-about-expected-exceptions) here as it's a little more concise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Originally I used pytest.raisesRegex but it broke the Python2 tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does the match argument to pytest.raises work with Python 2.7? e.g. pytest.raises(ValueError, match=r".* 123 .*").

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@nicholas-esterer nicholas-esterer merged commit a3550e6 into master Nov 5, 2020
@nicholas-esterer nicholas-esterer deleted the fig-set-subplots branch November 5, 2020 15:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

fig.set_subplots
3 participants