Conversation
adds a hovertemplate string @[omitempty] field to ScatterTrace in the plot module. Several trace types already expose Plotly’s hovertemplate (for example HeatmapTrace, ContourTrace, SurfaceTrace, ChoroplethTrace, ScatterMapboxTrace, and SankeyLink). This change brings the scatter trace in line with those APIs and with Plotly’s own scatter trace options. A small test (test_scatter_hovertemplate_in_traces_json) verifies that the hovertemplate field is emitted in the traces JSON when set.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughAdds a new public mutable string field Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plot/plot_test.v (1)
25-25: Make Line 25 assertion less brittle to JSON formatting differences.Exact substring matching with key+value formatting can break on serializer formatting changes (e.g., spacing). Consider splitting into key/value presence checks.
♻️ Suggested tweak
- assert traces.contains('"hovertemplate":"x=%{x}, y=%{y}<extra></extra>"') + assert traces.contains('"hovertemplate"') + assert traces.contains('x=%{x}, y=%{y}<extra></extra>')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@plot/plot_test.v` at line 25, The assertion on traces is brittle because it matches an exact JSON substring; instead update the test in plot_test.v to check the hovertemplate key and its value separately (e.g., assert traces.contains('"hovertemplate"') && traces.contains('x=%{x}, y=%{y}<extra></extra>')) or better: parse the JSON in traces and assert the hovertemplate field equals "x=%{x}, y=%{y}<extra></extra>" so spacing/formatting changes in serialization won't break the test; locate the assertion that uses traces.contains in the test and replace it with the split-presence checks or JSON-parse+field check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@plot/plot_test.v`:
- Line 25: The assertion on traces is brittle because it matches an exact JSON
substring; instead update the test in plot_test.v to check the hovertemplate key
and its value separately (e.g., assert traces.contains('"hovertemplate"') &&
traces.contains('x=%{x}, y=%{y}<extra></extra>')) or better: parse the JSON in
traces and assert the hovertemplate field equals "x=%{x}, y=%{y}<extra></extra>"
so spacing/formatting changes in serialization won't break the test; locate the
assertion that uses traces.contains in the test and replace it with the
split-presence checks or JSON-parse+field check.
adds a hovertemplate string @[omitempty] field to ScatterTrace in the plot module. Several trace types already expose Plotly’s hovertemplate (for example HeatmapTrace, ContourTrace, SurfaceTrace, ChoroplethTrace, ScatterMapboxTrace, and SankeyLink). This change brings the scatter trace in line with those APIs and with Plotly’s own scatter trace options. A small test (test_scatter_hovertemplate_in_traces_json) verifies that the hovertemplate field is emitted in the traces JSON when set.
Summary by CodeRabbit
New Features
Tests