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

add_{hline,vline,hrect,vrect} and add_{shape,image,annotation} to multiple facets, squashed #2840

Merged
merged 8 commits into from
Oct 22, 2020

Conversation

nicholas-esterer
Copy link
Contributor

Closes #2141 and closes #2140

See https://plotly.com/python/reference/layout/shapes/ for more information and chart attribute options!

<!-- #region -->
Copy link
Contributor

Choose a reason for hiding this comment

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

this "what about Dash" section gets added automatically at build time, and so should not be included here please

plot can be added via the `add_hline`, `add_vline`, `add_hrect`, and `add_vrect`
methods of `plotly.graph_objects.Figure`. These shapes are fixed to the
endpoints of one axis, regardless of the range of the plot, and fixed to data
coordinates on the other axis. For example
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
coordinates on the other axis. For example
coordinates on the other axis. Shapes added with these methods are added as [layout shapes](/python/shapes) (as shown when doing `print(fig)`, for example). For example

---

### Horizontal and Vertical Lines and Boxes (Autoshapes) in Plotly.py

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
*introduced in plotly 4.12*

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe also mention that users can pan and zoom, the shapes will adapt

@nicolaskruchten
Copy link
Contributor

Needs some changelog entries

@nicolaskruchten

This comment has been minimized.

@nicolaskruchten
Copy link
Contributor

Argh, please ignore last message :)

@nicolaskruchten
Copy link
Contributor

nicolaskruchten commented Oct 20, 2020

OK so this seems to add the shape to all subplots, even if they don't have traces on them:

import plotly.express as px

df = px.data.tips()
fig = px.scatter(df, x="total_bill", y="tip", color="sex", facet_row="time", facet_col="day")
fig.add_shape(x0=0, x1=1, y0=0, y1=1, type="line", row="all", col="all")
fig.show()

but the equivalent .add_hline() call does not... why?

@nicolaskruchten
Copy link
Contributor

The behaviour of add_trace() with either or both of row/col = "all" will need to:

  1. implement the "only on subplots with pre-existing traces" logic from .add_hline() and friends
  2. implement the logic we discussed out loud whereby by default all the traces are in the same legendgroup (some UID) unless legendgroup is explicitly set to None and all but the first trace have showlegend=False, unless showlegend is explicitly provided.

@nicolaskruchten
Copy link
Contributor

Ah, re point 2 above, we did discuss this and I think I had landed on the idea of documenting the following approach, which is a little verbose, but already works:

fig.add_trace(go.Scatter(x=[1,10], y=[1,5], legendgroup="hi", showlegend=False), row="all", col="all")
fig.data[-1].update(showlegend=True)

@nicolaskruchten
Copy link
Contributor

Re point 1 above and the same issue for add_shape ... we had discussed a kwarg exclude_empty_subplots but this doesn't seem to have survived in this PR :) Man, June seems far away now.

#### Adding Text Annotations to Autoshapes

Text can be added to an autoshape using the `annotation` keyword. Using the
above example:
Copy link
Contributor

Choose a reason for hiding this comment

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

could you please add a link to the annotations tutorial?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, sure! Is this how you do it? [annotations](/python/text-and-annotations)

Copy link
Contributor

@emmanuelle emmanuelle left a comment

Choose a reason for hiding this comment

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

I'm leaving small comments as I'm reading the code... I have not finished yet. The new domain positioning feature is really cool :-).

@@ -359,5 +359,8 @@ fig.show(config={'modeBarButtonsToAdd':['drawline',
]})
```


### Images Placed Relative to the Axes

Copy link
Contributor

Choose a reason for hiding this comment

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

placeholder for example here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, woops ;)

---

### Horizontal and Vertical Lines and Boxes (Autoshapes) in Plotly.py

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe also mention that users can pan and zoom, the shapes will adapt

df = px.data.tips()
fig = px.scatter(df, x="total_bill", y="tip", facet_row="smoker", facet_col="sex")
# Add annotations anchored to the top right corner of the resulting lines
fig.add_vline(x=30, line_color="purple", annotation=go.layout.Annotation(text="A"))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we discourage using go objects like this inside functions, so you could write dict(text="A") instead (@nicolaskruchten can you please confirm?)

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah let's use a dict here please.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wait, this was one of the specifications, that it should accept a go.layout.Annotation object, as well as a dict, this is to show that fact.

[
(
("all", [2, 4, 5], False),
_unzip_pairs([(r, c) for r in range(1, NROWS + 1) for c in [2, 4, 5]]),
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
_unzip_pairs([(r, c) for r in range(1, NROWS + 1) for c in [2, 4, 5]]),
zip(*itertools.product(range(1, NROWS + 1), [2, 4, 5]))

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the above does the same as your custom _unzip_pairs function (you'd have to import itertools as well, it's part of the standard library), so if it's possible to reduce the amount of home-written code it's better for maintenance and readability.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice, I like this trick!

@nicolaskruchten
Copy link
Contributor

nicolaskruchten commented Oct 21, 2020

So just to summarize what I think is missing here: the exclude_empty_subplots kwarg should exist in add_{shape|annotation|layout_image|trace|hline|vline|hrect|vrect} and should default to False for the methods that existed before and to True for the new methods.

In particular, added example for layout images, showing how their
position can reference an axis domain.
These will by default add their respective graph object to all subplots
but if exclude_empty_subplots is True, they will only add the graph
object to those subplots that already have traces on them.
single plot and so this returns False. """
return self._grid_ref is not None

def _subplot_contains_trace(self, xref, yref):
Copy link
Contributor

Choose a reason for hiding this comment

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

Right now a subplot with no traces but with a shape does render and "isn't empty" so I'm sort of thinking this should extend to shapes and annotations and images as well...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean we should use a function _subplot_contains_shape or something like that? Only add to plots with shapes already on them?

@nicolaskruchten
Copy link
Contributor

gotta fix the tests :)

@nicolaskruchten
Copy link
Contributor

💃 let's merge this thing!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants