Skip to content

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

Merged
PrimozGodec merged 2 commits intobiolab:masterfrom
PrimozGodec:fix-neighborus
Jul 10, 2019
Merged

[FIX] OWNeighbours fix manual apply for some options#3911
PrimozGodec merged 2 commits intobiolab:masterfrom
PrimozGodec:fix-neighborus

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