Skip to content

Provide element index with label_formatter#138

Merged
lucasmerlin merged 2 commits into
emilk:mainfrom
upsj:enumerated_label_formatter
Jun 26, 2026
Merged

Provide element index with label_formatter#138
lucasmerlin merged 2 commits into
emilk:mainfrom
upsj:enumerated_label_formatter

Conversation

@upsj

@upsj upsj commented Sep 26, 2025

Copy link
Copy Markdown
Contributor

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 former.

I changed the label_formatter interface to introduce three improvements:

  1. Instead of passing an empty string "" for the plot name when no data point is selected, we pass an enum
  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

@upsj

upsj commented Sep 26, 2025

Copy link
Copy Markdown
Contributor Author

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

upsj commented Sep 29, 2025

Copy link
Copy Markdown
Contributor Author

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

@upsj upsj force-pushed the enumerated_label_formatter branch from c7b927a to b9d844a Compare September 29, 2025 10:27
@upsj upsj force-pushed the enumerated_label_formatter branch from b9d844a to 4bb818a Compare June 23, 2026 07:40
@upsj upsj marked this pull request as ready for review June 23, 2026 07:40
@upsj

upsj commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

@emilk Rebased this to provide only a single interface, which makes it an interface break in the label formatter, but IMO a clear improvement removing the need for finding the nearest point twice.

@lucasmerlin lucasmerlin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, thanks!

@lucasmerlin lucasmerlin added enhancement New feature or request include in changelog This change will be included in the changelog labels Jun 26, 2026
@lucasmerlin lucasmerlin merged commit db52cb0 into emilk:main Jun 26, 2026
10 of 12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request include in changelog This change will be included in the changelog

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants