Skip to content

Clarify Lasso[Selector] demos #19221

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

Open
ianhi opened this issue Jan 3, 2021 · 5 comments
Open

Clarify Lasso[Selector] demos #19221

ianhi opened this issue Jan 3, 2021 · 5 comments

Comments

@ianhi
Copy link
Contributor

ianhi commented Jan 3, 2021

Problem

It is not easy to figure out the differences between the Lasso Demo and the Lasso Selector Demo. There is some redundancy between them

The Lasso Demo explanation contains this off putting sentence:

This is currently a proof-of-concept implementation (though it is usable as is). There will be some refinement of the API.

Furthermore, despite both Lasso and LassoSelector being in widgets.py the Lasso Demo is under the Event Handling sections of the gallery - which I find confusing.

https://matplotlib.org/3.3.2/gallery/widgets/lasso_selector_demo_sgskip.html and It not eas

Suggested Improvements

  1. Move Lasso Demo under the widgets examples
  2. remove the sentence about proof of concept implementation from Lasso Demo.
  3. Add a recommendation (and link to demo) to use LassoSelector for selecting points to the explanatory paragraph in Lasso Demo
    • Both examples should probably cross reference eachother actually
  4. Create an example showcasing the difference between Lasso and LassoSelector (After a cursory read I don't see why I wouldn't always prefer the latter)

Matplotlib version

  • Matplotlib documentation version: devdocs/3.3.3
@ianhi
Copy link
Contributor Author

ianhi commented Jan 3, 2021

This is currently a proof-of-concept implementation (though it is usable as is). There will be some refinement of the API.

Turns out this comment was added 15 years ago:
https://github.com/matplotlib/matplotlib/blame/7a0d65666ee9e73736500eddd20d65acafb86a65/examples/event_handling/lasso_demo.py#L10

at which point I have no doubt that it was a true statement. However, the api does seem to have solidified in the intervening years. Also in this rooting around through history it seems that LassoSelector was added (#730) several years after the initial Lasso implementation, is it always to be preferred?

@timhoffm
Copy link
Member

timhoffm commented Jan 4, 2021

Unfortunately, widgets are not very actively maintained. Without looking into the details, it may well be Lasso is superseded by LassoSelector. At least the limiting note in the Lasso example and the fact that the younger LassoSelector is more in line with the other selectors would make it plausible that LassoSelector was intended as a replacement. Unfortunately #730 is not explicit about it. Ping @efiring who merged #730: Do you remember anything about this?

@ianhi
Copy link
Contributor Author

ianhi commented Jan 4, 2021

Is this also a good first issue? Assuming my proposed changes are desireable some of them are low barrier to entry.

@timhoffm
Copy link
Member

timhoffm commented Jan 4, 2021

The difficult part is to decide what should be done. Once that's done it's possibly less work to directly do it than to write up what to do in a newcomer-friendly way. It's mostly getting the right wording in the right places.

@ianhi
Copy link
Contributor Author

ianhi commented Feb 14, 2023

Looking at this again due to #25155 and I now believe that Lasso should be deprecated in favor of LassoSelector

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants