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] Data Sets: Add filter #2695

Merged
merged 2 commits into from
Oct 24, 2017
Merged

[ENH] Data Sets: Add filter #2695

merged 2 commits into from
Oct 24, 2017

Conversation

lanzagar
Copy link
Contributor

Issue

Filtering of data sets was not supported.

Description of changes

Partially reuse filtering from GEO Data Sets (Orange-Bioinformatics). Includes pulling the class TokenListCompleter to core Orange and using it in owdatasets.

Includes
  • Code changes
  • Tests
  • Documentation

@kernc kernc added this to the 3.7 milestone Oct 20, 2017
Copy link
Contributor

@ales-erjavec ales-erjavec left a comment

Choose a reason for hiding this comment

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

self.completer = TokenListCompleter(
self, caseSensitivity=Qt.CaseInsensitive
)
self.filterLineEdit.setCompleter(self.completer)
Copy link
Contributor

Choose a reason for hiding this comment

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

The completer is not actually used? There is no completion model set after this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, you are right. I started moving functionality from GEO Datasets, but then changed to a simpler version. No caching from MySortFilterProxyModel and no completion. It actually seemed good enough for now. I haven't tested when caching would become important, but there are no performance problems for now (and probably won't be in the near future for a manually curated list of data sets). We might want to consider enabling/finishing completion (since it is already done in bioinf.), however, we can also go with a bare-bones approach (and remove moving that class for now) => less code, easier to maintain?
@ales-erjavec are the two related? Does adding a completer require the use of caching (probably should not be the case) or was there another reason for that in GEO (more data sets, automatically synced with a big external repository)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Are the two related? Does adding a completer require the use of caching.

No.

@codecov-io
Copy link

codecov-io commented Oct 23, 2017

Codecov Report

Merging #2695 into master will increase coverage by 0.06%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #2695      +/-   ##
==========================================
+ Coverage   75.83%   75.89%   +0.06%     
==========================================
  Files         338      338              
  Lines       59566    59594      +28     
==========================================
+ Hits        45173    45230      +57     
+ Misses      14393    14364      -29

@lanzagar lanzagar changed the title [RFC] Data Sets: Add filter [ENH] Data Sets: Add filter Oct 24, 2017
@astaric astaric merged commit 8b94e4c into biolab:master Oct 24, 2017
@lanzagar lanzagar deleted the datasets_filter branch March 14, 2022 14:41
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.

5 participants