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] Update output on new input, even if auto commit is disabled #3844

Merged
merged 10 commits into from
Jun 28, 2019

Conversation

janezd
Copy link
Contributor

@janezd janezd commented Jun 2, 2019

Issue

Fixes #3761.

Description of changes

Call unconditional_apply instead of apply; remove the pointless auto commit checkbox in box plot.

Includes
  • Code changes
  • Tests

@codecov
Copy link

codecov bot commented Jun 2, 2019

Codecov Report

Merging #3844 into master will increase coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3844      +/-   ##
==========================================
+ Coverage   84.94%   84.95%   +0.01%     
==========================================
  Files         376      376              
  Lines       70157    70220      +63     
==========================================
+ Hits        59595    59657      +62     
- Misses      10562    10563       +1

@ajdapretnar ajdapretnar self-assigned this Jun 3, 2019
@janezd
Copy link
Contributor Author

janezd commented Jun 3, 2019

I yesterday assigned this to @ajdapretnar; for this one, maybe @VesnaT would have to check it, too.

@ajdapretnar
Copy link
Contributor

I think considering this PR is dealing with apply, it would be nice if it fixed the mismatching buttons in Color and Concatenate, which are unlike those of other widgets.

@janezd
Copy link
Contributor Author

janezd commented Jun 5, 2019

This is one of supported generic options. It is more obvious than the usual (checkbox without a label) but we seldom use it because most widgets are not wide enough. I can of course remove it (the label is set by an explicit argument to gui.auto_commit, so I just have to remove the extra argument), but I'm undecided about whether the extra explanatory label indeed bothers me. @markotoplak, @irgolic, what are your opinions?

@ajdapretnar ajdapretnar removed their assignment Jun 7, 2019
@janezd janezd changed the title Update output on new input, even if auto commit is disabled [FIX] Update output on new input, even if auto commit is disabled Jun 19, 2019
@PrimozGodec
Copy link
Contributor

All widgets in this PR works correctly now, so I will merge it. With this PR debate about the inconsistency of auto-commit button started. I will open the separate issue to continue it.

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.

Widgets don't Apply, Send or Run when their input signals changed
4 participants