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

[FIX] OWNeighbours fix manual apply for some options #3911

Merged
merged 2 commits into from
Jul 10, 2019

Conversation

PrimozGodec
Copy link
Contributor

Issue

Manual apply didn't work for number of neighbors and Exclude rows. Those two fields were applied automatically even when Apply automatically unchecked.

Description of changes

Manual apply is enabled for number of neighbors and Exclude rows.

Includes
  • Code changes
  • Tests
  • Documentation

@codecov
Copy link

codecov bot commented Jun 28, 2019

Codecov Report

Merging #3911 into master will increase coverage by 0.4%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master    #3911     +/-   ##
=========================================
+ Coverage   84.32%   84.73%   +0.4%     
=========================================
  Files         385      371     -14     
  Lines       72825    64928   -7897     
=========================================
- Hits        61413    55015   -6398     
+ Misses      11412     9913   -1499

@janezd
Copy link
Contributor

janezd commented Jul 1, 2019

Good catch.

I replaced commit with a lambda function.

Some widgets have commit and some have apply, which is annoying enough because you never know what to call when working on different widgets. Now after this PR we would have apply and commit, which would have the same effect, but only after gui.auto_commit is called. This is even more confusing. :)

I think it's better to have either lambda or methods like on_n_neighbours_changed (which then call commit. Or apply. Whichever :)).

@PrimozGodec PrimozGodec changed the title [FIX] OWNeighbours enable manual apply for some options [FIX] OWNeighbours fix manual apply for some options Jul 10, 2019
@PrimozGodec PrimozGodec merged commit 8a01b2a into biolab:master Jul 10, 2019
@PrimozGodec PrimozGodec deleted the fix-neighborus branch July 10, 2019 09:16
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