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] Reset settings button #3795

Merged
merged 5 commits into from
May 31, 2019
Merged

Conversation

janezd
Copy link
Contributor

@janezd janezd commented May 10, 2019

Fixes #3564.

It would be nice if every widget had a reset settings button. This must be implemented specifically for each widget.

This PR prepares the GUI and implements the functionality for a single widget. If widget implements a method named reset_setting, a reset button will appear and call this method.

reset_settings will usually be simple: it must

  • copy the default settings (e.g. by calling the provided self._reset_settings),
  • resolving context related settings if needed (e.g. default for x and y in the scatter plot),
  • calling the necessary callbacks.

The icons is passable but could be improved upon. For one thing, it is a bit too large.

Includes
  • Code changes

@janezd janezd changed the title Reset settings button [WIP] Reset settings button May 16, 2019
@janezd janezd changed the title [WIP] Reset settings button Reset settings button May 17, 2019
@codecov
Copy link

codecov bot commented May 17, 2019

Codecov Report

Merging #3795 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #3795      +/-   ##
==========================================
+ Coverage   84.76%   84.76%   +<.01%     
==========================================
  Files         374      374              
  Lines       69148    69172      +24     
==========================================
+ Hits        58616    58637      +21     
- Misses      10532    10535       +3

@markotoplak
Copy link
Member

This restores last saved widget settings well. It should probably reset the settings to the defaults.

@markotoplak markotoplak requested review from VesnaT and removed request for VesnaT May 24, 2019 07:44
@markotoplak markotoplak removed their assignment May 24, 2019
@markotoplak markotoplak assigned markotoplak and unassigned janezd May 24, 2019
@markotoplak
Copy link
Member

@janezd, I added some tests. While I was writing them, pycharm suggested that I use resetSettings(), which seems to do something very similar to _reset_settings. Which one should we keep?

@janezd
Copy link
Contributor Author

janezd commented May 27, 2019

  • The widget gets a reset button if it implements reset_settings.
  • _reset_settings is a helper method (of OWComponent) that can be called by reset_settings.
  • resetSettings is a method that is apparently used only in tests for OWFeatureStatistics. As I see, it does not restore to original settings but probably to the last saved settings.

I would remove resetSettings.

janezd and others added 5 commits May 31, 2019 10:06
Method didn't really reset them (it reverted them to the saved state, not the
original), it wasn't used (except in a single place where it was unnecessary)
and, worst of all, its name was in camel case.

I believe the world is better without it.
@markotoplak markotoplak changed the title Reset settings button [ENH] Reset settings button May 31, 2019
@markotoplak markotoplak merged commit 4801e64 into biolab:master May 31, 2019
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.

Every widget should have a Reset Settings button
2 participants