Skip to content

Conversation

m-ad
Copy link
Contributor

@m-ad m-ad commented Oct 8, 2025

This PR extends the _mean() function in shapeannotation.py to allow for sequences of datetime objects as input. This fixes issue #3065, specifically it enables annotations on hline/vline objects that use datetime axes.

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 code generator and not the generated files.
  • I have added tests or modified existing tests.
  • For a new feature, I have added documentation examples (please see the doc checklist as well).
  • I have added a CHANGELOG entry if changing anything substantial.
  • For a new feature or a change in behavior, I have updated the relevant docstrings in the code.

@gvwilson gvwilson requested a review from emilykl October 9, 2025 12:12
@gvwilson gvwilson added feature something new P2 considered for next cycle community community contribution labels Oct 9, 2025
@gvwilson
Copy link
Contributor

gvwilson commented Oct 9, 2025

@emilykl please review

@emilykl
Copy link
Contributor

emilykl commented Oct 9, 2025

@m-ad Thank you for the PR!

I'm open to something along these lines as a "patch" fix to enable add_vline() and add_hline() to work with datetime axes, although as @nicolaskruchten mentioned, the "correct" fix would be to refactor the code such that we're not trying to do math with data values.

Ultimately, now that shape labels are natively supported in Plotly.js, I think the longer-term fix here is to refactor add_hline() and add_vline() to add annotation_text as a shape label rather than as a separate annotation; that will allow us to skip the step of computing averages of the data values. I mention it here just in case that's a task you'd be interested in -- I've opened issue #5373 to track.

Otherwise, something like what you've proposed here will work as a temporary fix.

There's a few issues with the logic as written; for example, falling back to returning x[0] is incorrect when x[0] and x[1] are not equal. We should simply raise an error in that case because we have no way of calculating an average.

That means cases like the reproducible example in #3065, where the date value is passed as a string, will require a different approach. I'm not sure exactly what the best approach is; trying to parse every value as a datetime seems a bit clunky, but technically speaking it will produce the correct result. Or maybe we just document this limitation and specify that if you are trying to use add_hline() or add_vline() for a datetime axis you must pass the value as a datetime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community community contribution feature something new P2 considered for next cycle

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants