Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Tooltip recipe for DataInspector #2095

Merged
merged 27 commits into from Oct 12, 2022
Merged

Add Tooltip recipe for DataInspector #2095

merged 27 commits into from Oct 12, 2022

Conversation

ffreyer
Copy link
Collaborator

@ffreyer ffreyer commented Jun 25, 2022

Description

This adds a tooltip(anchor, text) recipe which places some text in a box pointing to the anchor position. Here's an example with a few tooltips that I corrently have as a refimg test:

Screenshot from 2022-06-25 15-38-46

I'm planning for this to replace the tooltips created by DataInspector which should also make it easier to create persistent tooltips.

DataInspector:

Screenshot from 2022-06-25 18-06-08

Type of change

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected) because attribute names moved around and some got renamed?

Checklist

  • Added an entry in NEWS.md (for new features and breaking changes)
  • Added or changed relevant sections in the documentation
  • Added reference image tests for new plotting functions, recipes, visual options, etc.

TODO

  • switch to Tooltip in DataInspector
  • add white outline to text in DataInspector tooltips by default... maybe
  • figure out heatmap and image tooltips - either set background color and reset afterwards or add some other color indicator added back the old behavior (cell outline for heatmap, magnified pixel for image)
  • fix contourf tooltip alignment (and others if there are any)
  • fix broken volumeslices
  • restore boundingbox/selection indicators (with on/off switch)
  • fix duplication problems of some indicators
  • add attribute to shift tooltip pointer away from center

@MakieBot
Copy link
Collaborator

MakieBot commented Jun 25, 2022

Compile Times benchmark

Note, that these numbers may fluctuate on the CI servers, so take them with a grain of salt. All benchmark results are based on the mean time and negative percent mean faster than the base branch. Note, that GLMakie + WGLMakie run on an emulated GPU, so the runtime benchmark is much slower. Results are from running:

using_time = @ctime using Backend
# Compile time
create_time = @ctime fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @ctime Makie.colorbuffer(display(fig))
# Runtime
create_time = @benchmark fig = scatter(1:4; color=1:4, colormap=:turbo, markersize=20, visible=true)
display_time = @benchmark Makie.colorbuffer(display(fig))
using create display create display
GLMakie 13.77s (13.65, 13.91) 0.10+- 17.85s (17.64, 18.18) 0.18+- 16.81s (16.54, 17.07) 0.19+- 36.01ms (35.11, 36.58) 0.49+- 98.18ms (96.58, 99.74) 1.01+-
master 13.82s (13.65, 14.02) 0.17+- 17.80s (17.66, 17.91) 0.09+- 16.77s (16.47, 17.09) 0.22+- 36.67ms (35.98, 37.27) 0.46+- 97.53ms (96.09, 99.68) 1.23+-
evaluation -0.39%, -0.05s invariant (-0.38d, 0.50p, 0.14std) +0.30%, 0.05s invariant (0.37d, 0.51p, 0.14std) +0.20%, 0.03s invariant (0.16d, 0.77p, 0.21std) -1.82%, -0.66ms faster ✓ (-1.38d, 0.02p, 0.47std) +0.66%, 0.65ms invariant (0.58d, 0.30p, 1.12std)
CairoMakie 11.48s (11.24, 11.95) 0.27+- 17.93s (17.51, 18.98) 0.51+- 2.83s (2.75, 2.98) 0.09+- 33.53ms (31.33, 36.41) 1.52+- 23.96ms (23.64, 24.12) 0.18+-
master 11.38s (11.16, 11.76) 0.20+- 18.02s (17.43, 19.71) 0.81+- 2.84s (2.76, 3.00) 0.09+- 33.79ms (32.81, 35.83) 1.05+- 24.41ms (23.90, 25.61) 0.59+-
evaluation +0.90%, 0.1s invariant (0.43d, 0.44p, 0.24std) -0.55%, -0.1s invariant (-0.14d, 0.79p, 0.66std) -0.34%, -0.01s invariant (-0.11d, 0.84p, 0.09std) -0.78%, -0.26ms invariant (-0.20d, 0.72p, 1.29std) -1.91%, -0.46ms invariant (-1.05d, 0.09p, 0.39std)
WGLMakie 12.15s (12.09, 12.25) 0.05+- 26.06s (25.99, 26.12) 0.04+- 42.31s (42.05, 42.93) 0.29+- 34.90ms (34.20, 35.90) 0.66+- 1.66s (1.64, 1.68) 0.02+-
master 12.04s (11.98, 12.11) 0.04+- 26.05s (25.99, 26.10) 0.04+- 41.85s (41.70, 41.94) 0.08+- 34.80ms (34.29, 35.14) 0.34+- 1.67s (1.64, 1.71) 0.02+-
evaluation +0.97%, 0.12s slower X (2.39d, 0.00p, 0.05std) +0.04%, 0.01s invariant (0.28d, 0.61p, 0.04std) +1.08%, 0.46s slower X (2.14d, 0.01p, 0.19std) +0.29%, 0.1ms invariant (0.19d, 0.73p, 0.50std) -0.21%, -0.0s invariant (-0.17d, 0.75p, 0.02std)

@ffreyer
Copy link
Collaborator Author

ffreyer commented Jun 26, 2022

closes #2092

@github-actions
Copy link
Contributor

github-actions bot commented Jul 2, 2022

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 2, 2022

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

github-actions bot commented Jul 2, 2022

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@ffreyer
Copy link
Collaborator Author

ffreyer commented Jul 3, 2022

Here's a list of changed/moved attributes for di = DataInspector. Most of these can be set through keyword arguments in the constructor.

Styling attributes

Changed old new
x di.plot.text_padding di.plot.textpadding
x di.plot.text_align N/A
di.plot.textcolor di.plot.textcolor
di.plot.textsize di.plot.textsize
di.plot.font di.plot.font
x N/A di.plot.justification
x di.plot.background_color di.plot.backgroundcolor
di.plot.outline_color di.plot.outline_color
di.plot.outline_linestyle di.plot.outline_linestyle
x di.plot.outline_linewidth di.plot.outline_linewidth
m di.plot.indicator_color di.attributes.indicator_color
m di.plot.indicator_linewidth di.attributes.indicator_linewidth
m di.plot.indicator_linestyle di.attributes.indicator_linestyle
x di.plot.tooltip_offset di.attributes.offset (di.plot.offset)
x N/A di.attributes.enable_indicators
m di.plot.range di.attributes.range
m di.plot.depth di.attributes.depth

Notes:

  • text_align has more or less been replaced by justification
  • tooltip_offset has been replaced by offset::Real. To set it di.attributes.offset should be used. To adjust the final tooltip offset (i.e. for a custom show_data method) dio.plot.offset should be used.

Internal Attributes

Changed old new
m di.plot.enabled di.attributes.enabled
x di.plot.tooltip_align di.plot.placement
x di.plot._visible di.plot.visible
x di.plot._display_text di.plot.text
x di.plot._text_position di.plot[1]
x di.plot._px_bbox_visible di.attributes.indicator_visible
x di.plot._bbox_visible di.attributes.indicator_visible
x di.plot._bbox2D N/A
x di.plot._bbox3D N/A
x di.plot._model N/A
x di.plot._position N/A
x di.plot._color N/A

Notes:

  • tooltip_align has been replaced by placement which can be :left, :right, :above or :below.
  • The attributes from bbox2D to color were used as reusable Observables for indicators. Instead of those, attributes in di.temp_plots are now adjusted directly

@github-actions
Copy link
Contributor

github-actions bot commented Jul 4, 2022

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@ffreyer
Copy link
Collaborator Author

ffreyer commented Jul 27, 2022

New test image should produce
Screenshot from 2022-07-27 16-39-31

@ffreyer ffreyer marked this pull request as ready for review July 27, 2022 14:47
@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2022

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@ffreyer ffreyer mentioned this pull request Sep 9, 2022
5 tasks
@github-actions
Copy link
Contributor

github-actions bot commented Sep 9, 2022

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

Copy link
Member

@SimonDanisch SimonDanisch left a comment

Choose a reason for hiding this comment

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

Looks great! :) I just noticed that the tooltip starts as visible, which then hangs around in the bottom corner... Adding visible=false to tooltip in DataInspector seems to fix it without staying invisible, so I pushed that change!
I can update the refimages once CI is green

@github-actions
Copy link
Contributor

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2022

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

Changes:
- add `inspector_hover` attribute to allow replacing `show_data` on a per-plot basis
- add `inspector_label` attribute to allows for per-plot labels
- add `inspector_clear` attribute to allow for custom cleanup code (e.g. for an indicator visualization)
- adjust backends to ignore those attributes for rendering
- minor cleanup in `pick_sorted`
@ffreyer
Copy link
Collaborator Author

ffreyer commented Oct 8, 2022

I moved the changes from #2261 here since the history there got messed up and it's not that big of a change. As such this now closes #1759 and #1902.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2022

Missing reference images

Found 1 new images without existing references.
Upload new reference images before merging this PR.

@SimonDanisch SimonDanisch merged commit be31b12 into master Oct 12, 2022
@SimonDanisch SimonDanisch deleted the ff/Tooltip branch October 12, 2022 10:42
t-bltg pushed a commit to t-bltg/Makie.jl that referenced this pull request Dec 31, 2022
* add Tooltip recipe

* use Tooltip in DataInspector

* cleanup & restore useful indicators

* fix typo

Co-authored-by: Anshul Singhvi <asinghvi17@simons-rock.edu>

* fix tooltip alignment

* allow disabling of indicators

* cleanup

* update news

* add docs

* add test

* adjust tooltip args and fix indicator duplication

* fix indicator visibility & improve show_poly

* fix docs?

* fix error

* split news entry [skip ci]

* remove reused observables

* add align attribute

* fix typo

* start tooltip invisible

* manually merge MakieOrg#2261 (DataInspector callback attributes)

Changes:
- add `inspector_hover` attribute to allow replacing `show_data` on a per-plot basis
- add `inspector_label` attribute to allows for per-plot labels
- add `inspector_clear` attribute to allow for custom cleanup code (e.g. for an indicator visualization)
- adjust backends to ignore those attributes for rendering
- minor cleanup in `pick_sorted`

* update docs

* update NEWS

Co-authored-by: Anshul Singhvi <asinghvi17@simons-rock.edu>
Co-authored-by: Simon <sdanisch@protonmail.com>
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.

None yet

4 participants