plotly.py icon indicating copy to clipboard operation
plotly.py copied to clipboard

add_vline/hline annotation text location fixed to first shape if using a pre-instantiated dict for the "annotation" input parameter

Open Jamie-B22 opened this issue 4 years ago • 1 comments

Hi

When I instantiate a dict once with some constant formatting that I want to pass to the "annotation" argument for several calls of .add_vline() (or .add_hline()), all annotations for shapes created with this dict are placed with the shape created by the initial .add_vline() call.

e.g.:

import plotly.graph_objects as go

fig = go.Figure()

y_1 = [3000, 3500, 4000, 3500, 3000, 3500]
x_1 = list(range(len(y_1)))
fig.add_trace(go.Scatter(x=x_1, y=y_1))


annotation = dict(font_color="#000000")

fig.add_vline(x=2, annotation_text="vline 1", annotation=annotation)

print(f"annotation after first .add_vline():\n{annotation}")

fig.add_vline(x=3, annotation_text="vline 2", annotation=annotation)

print(f"annotation after second .add_vline():\n{annotation}")


fig.write_html("graph.html")

produces graph and output (similar result if .add_hline() used instead) - expected graph can be seen at the end of this post: image

annotation after first .add_vline():
{'font_color': '#000000', 'text': 'vline 1', 'xanchor': 'left', 'yanchor': 'top', 'x': 2, 'y': 1, 'showarrow': False}
annotation after second .add_vline():
{'font_color': '#000000', 'text': 'vline 2', 'xanchor': 'left', 'yanchor': 'top', 'x': 2, 'y': 1, 'showarrow': False}

Is this expected behaviour? As the x coordinate is explicitly given in the method I would expect that the annotation defined would be placed at that x, as happens with the different annotation_text values.

As we can see from the output, it is because the original annotation dictionary instance is being mutated in the process of adding the annotation. Then if an x value is present in the annotation dictionary, it is not overwritten, but the text value is if a new annotation_text value is passed.

Looking at the package code, I can achieve the behaviour I expected by implementing an else here: https://github.com/plotly/plotly.py/blob/cfad7862594b35965c0e000813bd7805e8494a5b/packages/python/plotly/plotly/shapeannotation.py#L203-L204 Suggestion:

    if annotation is None:
        annotation = dict()
    else:
        annotation = annotation.copy()

I have made this change locally and run fine and as I expect.

If the current behaviour is not the expected behaviour, and if the suggested change above is acceptable, I would be glad to implement and test it appropriately then submit a pull request.

Cheers!


Graph I expect: image

Jamie-B22 avatar Jan 14 '22 13:01 Jamie-B22

It just occurred to me that the fix I suggested above would still preserve the discrepancy in behaviour between not updating an x key value if it exists, but updating the text key value. It simply allows the annotation dict instance to be reused in a case like this.

It seems the discrepancy between how the x and text values are set in the method are from the lines below: text is set here: https://github.com/plotly/plotly.py/blob/cfad7862594b35965c0e000813bd7805e8494a5b/packages/python/plotly/plotly/shapeannotation.py#L205-L210

and x is set here: https://github.com/plotly/plotly.py/blob/cfad7862594b35965c0e000813bd7805e8494a5b/packages/python/plotly/plotly/shapeannotation.py#L223-L229

I'm not sure exactly what the comment is trying to say, but it seems to indicate that there is some doubt as to whether this is the best way to set the annotation dict items. Is there any reason that the if statement cannot be removed or changed to allow parameters explicitly passed to the .add_vline()/hline() methods to be reflected in the annotation dict here? Or should the go.layout.Annotation option suggested be implemented?

Jamie-B22 avatar Jan 14 '22 16:01 Jamie-B22