Skip to content

Conversation

@upsj
Copy link

@upsj upsj commented Sep 26, 2025

label_formatter only provides the name of the plot and the position of the point,
but that makes identifying the exact data point unnecessarily complicated,
when the index of the data point is available internally.

To change that, I wanted to add some more generic functionality, which either requires an interface break or adding a new function - I decided for the latter.

I added a new enumerated_label_formatter interface that introduces three improvements:

  1. Instead of passing an empty string "" for the label when no data point is selected, we pass an Option
  2. Instead of returning a String, we return an Option, which allows hiding the label when we don't need it, e.g. when not hovering over a data point.
  3. Instead of taking just the position and plot name, we take the position, data point index and plot name.
  • I have followed the instructions in the PR template

label_formatter only provides the name of the plot and the position of the point,
but that makes identifying the exact data point unnecessarily complicated,
when the index of the data point is available internally.

This adds a new enumerated_label_formatter interface that introduces three improvements:
1. Instead of passing an empty string "" for the label when no data point is selected, we pass an Option
2. Instead of returning a String, we return an Option<String>, which allows hiding the label when we don't need it,
   e.g. when not hovering over a data point.
3. Instead of taking just the position and plot name, we take the position, data point index and plot name.
@upsj
Copy link
Author

upsj commented Sep 26, 2025

There is perhaps a longer discussion to be had about the fact that the label_formatter has two purposes - providing a label in empty space, or near data points. Maybe an enum could be the better input here? (The naming is bad, but I also didn't think about it much)

enum HoverPosition {
    NearDataPoint {
        hover_position: Point,
        plot_name: &str,
        data_position: Point,
        data_index: usize
    },
    Elsewhere { hover_position: Point }
}

@upsj
Copy link
Author

upsj commented Sep 29, 2025

I replaced the Option usage by an enum, that makes the interface a bit more descriptive, albeit maybe also more verbose?

I'd love to get rid of the lifetime annotations, but my Rust fu isn't
good enough for that.
@upsj upsj force-pushed the enumerated_label_formatter branch from c7b927a to b9d844a Compare September 29, 2025 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant