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

Fix add_vline text annotation with dates #3731

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jjrob13
Copy link

@jjrob13 jjrob13 commented May 14, 2022

Background

I would like to use add_vline with a text annotation.

Closes: #3065

The workaround presented in #3065 (comment) breaks if people view the plot in a difference timezone than where it was generated.

As mentioned in that ticket, here is the error when data is used for text annotations TypeError: unsupported operand type(s) for +: 'int' and 'str'

The backtrace looks like:

> /lib/python3.9/site-packages/plotly/basedatatypes.py(4085)add_vline()
-> self._process_multiple_axis_spanning_shapes(
  /lib/python3.9/site-packages/plotly/basedatatypes.py(4031)_process_multiple_axis_spanning_shapes()
-> augmented_annotation = shapeannotation.axis_spanning_shape_annotation(
  /lib/python3.9/site-packages/plotly/shapeannotation.py(216)axis_spanning_shape_annotation()
-> shape_dict = annotation_params_for_line(
  /lib/python3.9/site-packages/plotly/shapeannotation.py(63)annotation_params_for_line()
-> eX = _mean(X)
  /lib/python3.9/site-packages/plotly/shapeannotation.py(7)_mean()
-> return float(sum(x)) / len(x)

For line annotations, the same x value is passed for both x1/x2, so the mean is trivial to compute.

4084        ):
4085 ->         self._process_multiple_axis_spanning_shapes(
4086                dict(type="line", x0=x, x1=x, y0=0, y1=1),
4087                row,
4088                col,
4089                "vline",
4090                exclude_empty_subplots=exclude_empty_subplots,
-> return float(sum(x)) / len(x)
(Pdb) p len(x)
2
(Pdb) p x[0] == x[1]
True

Code PR

  • 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.
  • 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.
  • For a new feature or a change in behaviour, I have updated the relevant docstrings in the code to describe the feature or behaviour (please see the doc checklist as well).

Test Plan

I'm now able to use add_vline and text annotations with dates. The repro in #3065 works now

@jjrob13 jjrob13 changed the title Fix add_vline with text annotation Fix add_vline text annotation with dates May 14, 2022
@jjrob13
Copy link
Author

jjrob13 commented Dec 21, 2022

cc: @nicolaskruchten this is a small bugfix that is useful for us, and it looks like people have been reporting it a few times (see PR description for issue)

@gvwilson gvwilson self-assigned this Jul 5, 2024
@gvwilson gvwilson removed their assignment Aug 2, 2024
@gvwilson gvwilson added P2 considered for next cycle fix fixes something broken labels Aug 12, 2024
@gvwilson
Copy link
Contributor

@marthacryan can you please have a look at this one (and if possible add a test when merging) thanks - @gvwilson

@adityaraute
Copy link

As this comment (#3065 (comment)) mentions, I would suggest checking for types would be a better way than making workaround fixes to those methods.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix fixes something broken P2 considered for next cycle
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error on using add_vline with text annotation for data with date-time x axis
4 participants