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] Rank widget computation in a separate thread #4908

Merged
merged 2 commits into from
Oct 6, 2020

Conversation

PrimozGodec
Copy link
Contributor

Issue

Rank widget blocks Orange while computing on a larger dataset

Description of changes

Use ConcurrentWidgetMixin in the rank widget.

Includes
  • Code changes
  • Tests
  • Documentation

@PrimozGodec PrimozGodec force-pushed the rank-concurent branch 3 times, most recently from f44a0bc to 9adddfa Compare July 17, 2020 12:13
@codecov
Copy link

codecov bot commented Jul 17, 2020

Codecov Report

Merging #4908 into master will increase coverage by 0.00%.
The diff coverage is 95.34%.

@@           Coverage Diff           @@
##           master    #4908   +/-   ##
=======================================
  Coverage   84.62%   84.62%           
=======================================
  Files         285      285           
  Lines       59427    59462   +35     
=======================================
+ Hits        50288    50319   +31     
- Misses       9139     9143    +4     

@markotoplak
Copy link
Member

@PrimozGodec, please make sure that run does use any attributes from the object, because they can now be set independently (connecting a new data set to it could do that).

As it is now, run() calls get_scorer_score and get_method_scores which both use self.data. To be safe, could you make run() and all the methods it calls static.

@markotoplak markotoplak marked this pull request as draft September 25, 2020 12:36
@PrimozGodec PrimozGodec force-pushed the rank-concurent branch 2 times, most recently from a65ad60 to a73c5e2 Compare September 28, 2020 10:42
@PrimozGodec PrimozGodec marked this pull request as ready for review September 28, 2020 10:56
@PrimozGodec
Copy link
Contributor Author

@markotoplak I changed now such that problematic methods get data now and are static. lru_cache uses hashes of arguments to store results.

@PrimozGodec PrimozGodec force-pushed the rank-concurent branch 8 times, most recently from e343016 to 194f4a8 Compare October 2, 2020 11:58
@PrimozGodec
Copy link
Contributor Author

Documentation test will pass after merging this PR #5010

I hope it is working now correctly.

Because it timeouted once. I decreased duration of 3 secs to 1 sec.
@markotoplak markotoplak merged commit 3a6e272 into biolab:master Oct 6, 2020
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.

2 participants