Skip to content

FIX: Handle hidden annotations during deletion in mpl plot#13703

Open
DerAndereJohannes wants to merge 1 commit intomne-tools:mainfrom
DerAndereJohannes:mpl_fix_annotation_removal
Open

FIX: Handle hidden annotations during deletion in mpl plot#13703
DerAndereJohannes wants to merge 1 commit intomne-tools:mainfrom
DerAndereJohannes:mpl_fix_annotation_removal

Conversation

@DerAndereJohannes
Copy link
Contributor

Reference issue

Fixes #13511.

What does this implement/fix?

This pull request fixes an issue in which removing annotations from the raw matplotlib figure would remove additional unintended annotations.

The issue with the current live code is that it assumes that the visible annotations are all grouped together in a continuous block starting from offset. However, this logic does not hold up.

For example, if is_onscreen is [True, False, True], then the offset is 0. Therefore zorders[0:2] get the visible_zorders instead of index 0 and 2.

The fix simply masks the zorders using the is_onscreen to assign visible_zorders to the correct indices.

Additional information

I have just implemented the fix here without a new regression pytest. Let me know how I should implement one if it is needed. Thank you!

Copy link
Member

@larsoner larsoner left a comment

Choose a reason for hiding this comment

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

Code looks good to me. Any way you could update a test that would fail on main but pass on this PR? Maybe somewhere in

def test_plot_annotations(raw, browser_backend):

And you can look at _annotation_helper to see some logic/magic to interact with the window using code.

It'll be a pain (harder than implementing the actual fix!) but I fear without this we can too easily regress in the future

@DerAndereJohannes
Copy link
Contributor Author

No worries, I will give it a shot! Thank you for the hints.

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.

Dissapearing annotations

2 participants