Skip to content

Improve parameter fixed checkboxes#32

Merged
pavoljuhas merged 6 commits intodiffpy:maintfrom
pavoljuhas:improve-parameter-fixed-checkboxes
Dec 14, 2018
Merged

Improve parameter fixed checkboxes#32
pavoljuhas merged 6 commits intodiffpy:maintfrom
pavoljuhas:improve-parameter-fixed-checkboxes

Conversation

@pavoljuhas
Copy link
Copy Markdown
Member

This improves handling of the fixed checkboxes in the Parameters panel,
especially when several cells are selected for the Fixed / Free context menu.

Simplify handler for popup-fix/free menu on Parameters panel.
Remove old ValueError block as it should not happen.
Allow selecting several cells in the "fixed" column without
flipping their value.
Do not change focused cell when flipping one checkbox.
@codecov
Copy link
Copy Markdown

codecov Bot commented Dec 14, 2018

Codecov Report

Merging #32 into maint will increase coverage by 2.63%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##            maint      #32      +/-   ##
==========================================
+ Coverage   25.22%   27.86%   +2.63%     
==========================================
  Files          88       89       +1     
  Lines       11634    11717      +83     
==========================================
+ Hits         2935     3265     +330     
+ Misses       8699     8452     -247
Impacted Files Coverage Δ
src/diffpy/pdfgui/tests/TestParametersPanel.py 100% <100%> (ø)
src/diffpy/pdfgui/gui/parameterspanel.py 54.5% <100%> (+54.5%) ⬆️
src/diffpy/pdfgui/gui/errorreportdialog.py 22.01% <0%> (+22.01%) ⬆️
src/diffpy/pdfgui/gui/errorwrapper.py 38.46% <0%> (+38.46%) ⬆️
...fpy/pdfgui/gui/wxExtensions/autowidthlabelsgrid.py 66.66% <0%> (+66.66%) ⬆️
src/diffpy/pdfgui/gui/debugoptions.py 71.42% <0%> (+71.42%) ⬆️
src/diffpy/pdfgui/gui/pdfpanel.py 73.33% <0%> (+73.33%) ⬆️
src/diffpy/pdfgui/gui/pdfguiglobals.py 91.66% <0%> (+91.66%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fe47600...d9d39c6. Read the comment docs.

@pavoljuhas
Copy link
Copy Markdown
Member Author

@dragonyanglong - can you please test the new behavior of Parameters Panel checkboxes and post any problems or comments here?
If you can test on either of Linux or Windows platforms it would be really helpful.

Copy link
Copy Markdown
Collaborator

@dragonyanglong dragonyanglong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @pavoljuhas , sure! Could you tell me what are the usecases to test this PR?

BTW, I am testing on linux system and I found one usecase as below, not sure is that related with this PR or not.
Step 1. In the beginning, the initial parameter panel looks like this:
image

Step 2. Select the refined column, put mouse on the parameter 2 row box, right click - click fix/free, it would look like this. currently, the PR doesn't make difference yet.
image
image

Step 3. Again put mouse on the parameter 2 row box, right click - click fix/free, now the PR makes difference.

In this PR with newest commits, it would be all rows being "fixed":
image

But in older version PDFgui without this PR, only the last three rows would be "fixed":
image

@pavoljuhas
Copy link
Copy Markdown
Member Author

@dragonyanglong - thank you for quick and detailed response.

The PR should fix the following misbehaviors of the Fixed-column checkboxes.

missed clicks when flipping one value

  • deselect all checkboxes and then use arrow keys to select R1 C2
  • click two times R2 C2 with about 2 second delay. The first click flips on, but the second click is ignored. It takes the 3rd click to flip checkbox off.

undesired flips when selecting parameters for "Fix / Free" menu

  • click R1 C1 then shift-click R3 C2. The checkbox in third row flips, but it would be more intuitive if it remained the same.

the Fix/Free behavior above

  • "Fix / Free" menu should set the same flag in all selected rows, but it is somehow broken as you observed.

@dragonyanglong
Copy link
Copy Markdown
Collaborator

Thanks @pavoljuhas for the instructions. I finished the testing on linux Ubuntu 18.04.1 LTS and Mac macOS Sierra 10.12.4. Sorry I don't have windows computer.

  • missed clicks when flipping one value
  • deselect all checkboxes and then use arrow keys to select R1 C2
  • click two times R2 C2 with about 2 second delay. The first click flips on, but the second click is ignored. It takes the 3rd click to flip checkbox off.
  • comment: in macOS system, I noticed an additional misbehavior related to this. After finishing the two steps above, use arrow keys to deselect R1 C2, it will perform like clicked twice (equivalent as not changing anything). This PR fixes the problem.
  • undesired flips when selecting parameters for "Fix / Free" menu
  • click R1 C1 then shift-click R3 C2. The checkbox in third row flips, but it would be more intuitive if it remained the same.
  • the Fix/Free behavior above
  • "Fix / Free" menu should set the same flag in all selected rows, but it is somehow broken as you observed.

In addition, the src/diffpy/pdfgui/gui/parameterspanel.py is not consistent with wxglade-generated code, which will be fixed by issue #31 (Sorry I didn't find time to work on that issue yet, I will do in near future, sorry about the delay)

Overall, the PR looks good to me. Thanks Pavol!

- provide DISPLAY on Linux via xvfb-run
- execute with Python.app on Mac OS X
- add python-wxtools to test with system Python
@pavoljuhas pavoljuhas merged commit d9d39c6 into diffpy:maint Dec 14, 2018
pavoljuhas added a commit that referenced this pull request Dec 14, 2018
* take empty parameter-fixed value as False
* improve behavior of parameter-fixed checkboxes
* add tests to cover changed methods
* fix spurious call to needsSave
* support execution of wx GUI on travis

Closes #32.
@pavoljuhas
Copy link
Copy Markdown
Member Author

@dragonyanglong - thank you for checking!

@pavoljuhas pavoljuhas deleted the improve-parameter-fixed-checkboxes branch December 14, 2018 22:26
pavoljuhas added a commit that referenced this pull request Jul 12, 2019
* merge branch 'improve-parameter-fixed-checkboxes'

Incorporate #32.
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