Skip to content

Conversation

@ajaust
Copy link

@ajaust ajaust commented Jan 20, 2026

This ensures that the data presented to the user is identical with the data exported. It follows the same approach how other tables in ERT are made non-editable.

I was considering to add a test for this, but it seems that there are no such tests for the other tables. I also found some pitfalls w.r.t. tests:

  • Testing for the NoEditTriggers property in the test does not actually reproduce the behavior of user when interacting with the GUI.
  • Other non-editable tables don't seem to have tests about for whether they are editable or not.
  • We could implement non-editable tables by removing the ItemIsEditable flag from each model describing a table cell instead of removing edit triggers. In this case the test would have to be rewritten.
  • I did not find a way to programmatically edit the table cells using the test framework that reliably reproduce the user behavior. without using methods that rely on the event loop, e.g., imitating mouse clicks and button presses.
    • Imitating mouse clicks and button presses etc. depends on the event loop which can lead to flaky tests. The pytest-qt package advises to avoid actions depending on the event loop.
    • Using the programmatic approach, e.g., using setText is not covered by the event triggers which means that setText succeeds even if the cell itself is non-editable.

Issue
Resolves #12574

Approach
The triggers to edit the QTableWidget are removed. It follows the same approach how other tables in ERT are made non-editable.

This video should show that the clicking/double clicking on the grid cell does not enter the "Edit" mode of the cell. I don't know if there is a better way to show that the cell is not editable.

Screen.Recording.2026-01-20.at.15.05.40.mov
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Added appropriate release note label
  • Commit history is consistent and clean, in line with the contribution guidelines.
  • Make sure unit tests pass locally after every commit (git rebase -i main --exec 'just rapid-tests')

When applicable

  • When there are user facing changes: Updated documentation
  • New behavior or changes to existing untested code: Ensured that unit tests are added (See Ground Rules).
  • Large PR: Prepare changes in small commits for more convenient review
  • Bug fix: Add regression test for the bug
  • Bug fix: Add backport label to latest release (format: 'backport release-branch-name')

This ensures that the data presented to the user is identical with the
data exported. It follows the same approach how other tables in ERT are
made non-editable.
@ajaust ajaust added the release-notes:bug-fix Automatically categorise as bug fix in release notes label Jan 20, 2026
@codecov-commenter
Copy link

codecov-commenter commented Jan 20, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.64%. Comparing base (b34e318) to head (cbc1b3f).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #12688      +/-   ##
==========================================
- Coverage   90.66%   90.64%   -0.02%     
==========================================
  Files         429      430       +1     
  Lines       29808    29815       +7     
==========================================
+ Hits        27024    27027       +3     
- Misses       2784     2788       +4     
Flag Coverage Δ
cli-tests 37.59% <0.00%> (+0.03%) ⬆️
gui-tests 69.33% <100.00%> (-0.01%) ⬇️
performance-and-unit-tests 73.92% <0.00%> (-0.02%) ⬇️
test 38.06% <0.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@codspeed-hq
Copy link

codspeed-hq bot commented Jan 20, 2026

CodSpeed Performance Report

Merging this PR will not alter performance

Comparing ajaust:fix-parameters-table-editable (cbc1b3f) with main (7238d60)

Summary

✅ 22 untouched benchmarks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

release-notes:bug-fix Automatically categorise as bug fix in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

User can edit parameters table in Manage Experiments, but changes are not exported

2 participants