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

[ENH] Match Keywords in Widget Search #3117

Merged
merged 4 commits into from
Jul 23, 2018
Merged

Conversation

irgolic
Copy link
Member

@irgolic irgolic commented Jul 4, 2018

Issue

At the moment, searching for widgets only queries widget titles. This leads to one having to type the widget's exact name to find it (e.g. "k-means"). With searching by keyword, widgets can be assigned other tags (e.g. "means", "kmeans").

Description of changes

Overriding filterAcceptsRow() in quick menu, matching both keywords and widget title.

Includes
  • Code changes
  • Tests
  • Documentation

@irgolic
Copy link
Member Author

irgolic commented Jul 4, 2018

Note: Most widgets currently do not hold keyword data.

@irgolic irgolic changed the title [WIP] Match Keywords in Widget Search Match Keywords in Widget Search Jul 4, 2018
@markotoplak
Copy link
Member

A very important improvement. Thank you for working on it!

Copy link
Contributor

@lanzagar lanzagar left a comment

Choose a reason for hiding this comment

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

I started writing some suggestions for individual keywords, but will stop for now.
I think the functionality is a very good enhancement, and we can go over the list of actual keywords in another PR. Maybe after some discussion with others.

The only drawback I see to this approach is that currently the matches in names and keywords are treated equally so this can make it harder to get to a specific widget for which I know the name. The other PR (#3112) which addresses sorting of suggestions could prevent this, by sorting all the exact matches above keyword matches.

@@ -27,6 +27,7 @@ class OWConcatenate(widget.OWWidget):
description = "Concatenate (append) two or more datasets."
priority = 1111
icon = "icons/Concatenate.svg"
keywords = ["append", "join"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe "extend" as well

@@ -27,6 +27,7 @@ class OWMergeData(widget.OWWidget):
description = "Merge datasets based on the values of selected features."
icon = "icons/MergeData.svg"
priority = 1110
keywords = []
Copy link
Contributor

Choose a reason for hiding this comment

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

you could add "join" here as well, like for Concatenate

@@ -13,7 +13,7 @@ class OWSave(widget.OWWidget):
description = "Save data to an output file."
icon = "icons/Save.svg"
category = "Data"
keywords = ["data", "save"]
keywords = ["save"]
Copy link
Contributor

Choose a reason for hiding this comment

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

this is already part of the name isn't it

@@ -103,6 +103,7 @@ class OWSelectAttributes(widget.OWWidget):
"data features, classes or meta variables."
icon = "icons/SelectColumns.svg"
priority = 100
keywords = ["attributes"]
Copy link
Contributor

Choose a reason for hiding this comment

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

if attributes, then maybe also "variables" and "features"
but "filter" is probably something users would try too

@@ -76,6 +76,7 @@ class OWSelectRows(widget.OWWidget):
icon = "icons/SelectRows.svg"
priority = 100
category = "Data"
keywords = []
Copy link
Contributor

Choose a reason for hiding this comment

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

"filter" ?

@@ -13,6 +13,7 @@ class OWConstant(OWBaseLearner):
"Orange.widgets.regression.owmean.OWMean",
]
priority = 10
keywords = []
Copy link
Contributor

Choose a reason for hiding this comment

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

"majority" and "mean"

@@ -22,6 +22,7 @@ class OWLinearRegression(OWBaseLearner):
"Orange.widgets.regression.owlinearregression.OWLinearRegression",
]
priority = 60
keywords = []
Copy link
Contributor

Choose a reason for hiding this comment

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

"ridge", "lasso", "elastic net"

@@ -90,6 +90,7 @@ class OWKMeans(widget.OWWidget):
"quality estimation."
icon = "icons/KMeans.svg"
priority = 2100
keywords = ["means", "kmeans"]
Copy link
Contributor

Choose a reason for hiding this comment

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

"means" is unnecessary as that worked before keywords already... The names are split into words so k-Means matched k and Means (and prefixes).
I would add "clustering" however.

@lanzagar
Copy link
Contributor

lanzagar commented Jul 5, 2018

Another comment: a commit like 6d5f8c3 (reimplemented title matching) is usually not useful to have in the git history. When fixing/improving/reworking your own changes you should squash the commits into one containing the final version.

@markotoplak
Copy link
Member

I see that to some widgets empty keywords were added. Widgets should also work properly if the keywords were not defined (as it is the case of most current add-on).

Commits with undefined keywords (keywords = []) should be removed from this PR.


# match name and keywords
accepted = False
for keyword in [name] + keywords:
Copy link
Contributor

Choose a reason for hiding this comment

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

How about also iterating over filter(None, re.split(r'\W+', description.description.lower()))? This way, many widgets won't even need separate keywords, e.g. K-means already has "clustering", "silhouette", ...

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd rather manually set keywords to be able to finely control what shows up in search, avoiding the overcrowding of search results. Each widget shouldn't need more than a few keywords I think.

@irgolic
Copy link
Member Author

irgolic commented Jul 5, 2018

To clarify, widgets work properly without keywords defined (even without an empty list of keywords). I just added an empty list in case someone notices it in the future and thinks of something good to fill it with.

@lanzagar lanzagar changed the title Match Keywords in Widget Search [ENH] Match Keywords in Widget Search Jul 23, 2018
@lanzagar lanzagar merged commit dc51c3d into biolab:master Jul 23, 2018
@lanzagar lanzagar added this to the 3.15 milestone Jul 24, 2018
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.

4 participants