Add Cochran rule indicator to Sieve + tests #7150
Conversation
Implemented a Cochran condition check based on expected frequencies in the Sieve Diagram widget. A small green or red square is displayed next to the "Cochran:" label to indicate whether the rule is satisfied. This provides a quick, language-independent visual cue without altering the existing color scheme of the diagram.
…gram 2nd change for add tests Implemented a Cochran condition check based on expected frequencies in the Sieve Diagram widget. A small green or red square is displayed next to the "Cochran:" label to indicate whether the rule is satisfied. This provides a quick, language-independent visual cue without altering the existing color scheme of the diagram.
2nd change: add Cochran rule check with visual indicator to Sieve Dia…
Introduced a new unit test that checks the Cochran’s rule indicator in the Sieve widget. The test uses a balanced 3x3 contingency table, where all expected frequencies are greater than 5, ensuring that the Cochran condition is satisfied. This verifies that the widget correctly sets the internal flag and displays the green indicator.
Add test for Cochran indicator when condition is satisfied
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #7150 +/- ##
==========================================
+ Coverage 88.78% 88.79% +0.01%
==========================================
Files 334 335 +1
Lines 73700 73876 +176
==========================================
+ Hits 65434 65600 +166
- Misses 8266 8276 +10 🚀 New features to boost your workflow:
|
This commit updates and refines the Cochran indicator logic added in the previous patch: Groups the label and square into a single QGraphicsItemGroup for consistent positioning. Ensures old indicators are properly removed before drawing new ones. Uses cosmetic pens to avoid scaling artifacts on HiDPI displays. Adds defensive checks to keep the indicator inside the scene bounds. Improves robustness so tests and different rendering backends behave consistently. This is a follow-up to the earlier patch that introduced the Cochran rule indicator, addressing review feedback and making the feature more reliable.
Refine Cochran indicator added in previous patch
Adjusted bottom margin so the Cochran indicator is rendered correctly and does not get clipped when resizing the window.
…in-Sieve-widget Fix Cochran indicator positioning in Sieve widget
Cleaned up trailing spaces to improve code style consistency and pass linting checks.
Remove trailing whitespace in test_owsieve.py
- Replaced redundant list comprehensions with simpler list() calls or slicing. - Switched from dict() constructor to dictionary literal for readability. - Renamed imported 'filter' to avoid redefining the built-in name. These are code quality improvements only, with no functional changes.
Refactor comprehensions, dict creation, and rename 'filter' import
- Added new tests to cover Cochran’s rule indicator (pass and fail cases). - Extended test_input_features to verify attribute resolution. - Added test for update_selection to ensure selected and annotated outputs are produced.
Update and extend Sieve widget tests
|
Hi, just to clarify the current state of this PR: When I opened it, I started getting CI errors from other parts of the codebase not related to my changes. While I tried to fix them, it created even more unrelated warnings and errors, although I believe the core changes I introduced are correct (example: refactor comprehensions, dict creation and filter import). Regarding translations: there is only one new string, currently "Cochran:". This should more precisely be "Cochran’s rules:", and ideally it should go through the i18n system. If you prefer, I can close this PR and reopen a clean one with only the original intended changes, without the extra attempts to fix unrelated issues. Thanks for the review and guidance! |
|
@gmolledaj, thanks for your patience. Some tests crash at random; this is annoying but impossible to debug and fix. We would, hopefully, one day. For now, ignore them. As for Slovenian translations, it obviously has to be done by somebody else (probably: me) after @thocevar or I review your PR. (We were the obvious choice so we volunteered, but we're both too busy right now.) |
Thanks for the update! I completely understand that you’re busy right now. |
|
Warning the user about inadequate data size is a good idea, but the meaning of the red/green square is not self evident. I understand your wish to make it language-independent --- but I don't think it works; there has to be some text to explain it. @thocevar proposed to make this a warning in the status line, e.g. and the you can trigger it with Doing it this way is more in line with other Orange widgets, and it also doesn't take additional space. It also simplifies the code because you don't have to paint the square, and you don't need the extra attribute |
- Added _cochran_ok() method to evaluate Cochran’s rule - Show Information message when the rule is met - Show Warning message when the rule is not met
Cochran with message Warning and Information
- Added tests with balanced and unbalanced contingency tables - Verify that Information is shown when the rule is met - Verify that Warning is shown when the rule is not met
Add tests for Cochran’s rule messages in Sieve Diagram
Issue NONE
Description of changes
This pull request introduces the following changes:
Cochran’s rule indicator in Sieve widget
Adds a small green/red square below the χ² and p-value labels.
The indicator turns green when Cochran’s rule is satisfied and red otherwise.
A new internal attribute (_cochran_ok) stores the evaluation result.
Unit tests
Added tests to ensure the indicator behaves correctly:
test_cochran_indicator_passes checks that the indicator is green when conditions are satisfied.
test_cochran_indicator_fails checks that the indicator is red when conditions are violated.
These changes improve the statistical robustness of the Sieve visualization and provide users with a quick visual check of Cochran’s rule.
Includes