-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
add_shape, add_layout_image, add_annotation for multiple facets at once, add_hline and add_vline (+add_vrect/add_hrect) #2558
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
Conversation
Need more comprehensive tests for all the other add_* functions and for all the possible combinations of row and col specification.
Simply pass a tuple for row and/or col where the second arg == 'all'.
(maybe this is overkill though, as a user could easily pass list(range(...)) instead)
|
||
# Get grid_ref if specific row or column requested | ||
if row is not None: | ||
grid_ref = self._validate_get_grid_ref() | ||
refs = grid_ref[row - 1][col - 1] | ||
# TODO It is assumed that grid_ref has an equal number of columns in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this can be not-true, you can have really odd layouts: https://plotly.com/python/subplots/#multiple-custom-sized-subplots
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey so I checked this out and it turns out even in this case grid_ref
is still a rectangle, just some of the entries are None. So we could still easily iterate through the grid_ref
using something like
for r in range(1,len(grid_ref)+1):
for c in range(1,len(grid_ref[0])+1):
sub_plot=fig.get_subplot(row=r,col=c)
if sub_plot is not None:
....
We just check if it is not None before doing anything to it 🔧
Not yet tested.
I greatly refactored the old approach and added basically this one function (it calls some helpers which you can check out in the code):
The idea is to change |
|
Recursion is only of depth 1.
…w and col Still need to add the product argument though.
We have decided to not support the cartesian product mode for |
for the add_trace and add_{shape,layout_image,annotation} Figure methods.
Hmm seeing now that the |
Ok so the PR doesn't affect |
This makes it possible to replace a shape in the layout with one where its one axis is fixed to the paper. This makes it possible to easily implement functions like add_vline, which add a horizontal line to a plot.
Still need to write tests though.
No tests written yet
For reviewers: |
@@ -8,6 +8,8 @@ | |||
import warnings | |||
from contextlib import contextmanager | |||
from copy import deepcopy, copy | |||
import itertools | |||
import pdb |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this probably shouldn't be here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(the pdb
part I mean)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
itertools yes, pdb no
I think reusing |
Making sure that position is for any orientation of the shape.
They can be added using the annotation keyword argument: fig.add_hline(y=1,annotation=go.layout.Annotation(text="example 1")) or you can specify their parameters using the annotation_ prefixed keywords fig.add_hline(y=1,annotation_text="example 2") There's also a quick way to specify some commonly encountered positions, through the annotation_position argument fig.add_vrect(y=1,annotation_text="example 3", annotation_position="inside bottom left")
Moved the tests for axis spanning shapes and their annotated versions to test_autoshapes.
hline, vline, hrect, vrect, can be annotated. To see an example of what this looks like, you can run from the root of this repo: VISUALIZE=1 python packages/python/plotly/plotly/tests/test_core/test_autoshapes/test_annotated_shapes.py Note: probably commit 8a62ba7 has to get checked out before you can do this. |
Made common file to share functions for all tests Wrote spec of annotation shape tests.
See packages/python/plotly/plotly/tests/test_core/test_autoshapes/test_annotated_shapes.py for information on how to configure the test (it may need to be updated from time to time).
Annotations look great! Some comments:
|
…shapes do not try and set the most recently added layout object's xref and yref if we wanted to add it to a plot with one subplot/facet.
Updated regex in EnumeratedValidator.build_regex_replacement
Before Python3.5 it seems keys were not sorted by default (although json.dumps says they always aren't by default, but this might be related to some other serialization).
But if we say |
That sounds good |
Fails on bad description.
how close are the docs to being done here? Need any help or guidance? |
Just have to change them to reflect the behaviour where specifying no row or col puts the shapes on all subplots. And I have to document the |
Actually I was mistaken, it is sufficient to have the documentation appear when calling |
No, we need to have docs written in the MD files :) Also: the build output from |
OK, so once we release plotly.js 1.57.0, i'll update |
Closes #2141 and closes #2140
Making it possible to specify multiple rows and columns when using
add_shape
,add_layout_image
oradd_annotation
.This pull request is to share the work, so a lot is missing still.
Code PR
plotly.graph_objects
, my modifications concern thecodegen
files and not generated files.modified existing tests.
new tutorial notebook (please see the doc checklist as well).