fix: crash when omega fit fails alongside other scale coefficients - #344
fix: crash when omega fit fails alongside other scale coefficients#344juliuspfadt wants to merge 1 commit into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #344 +/- ##
==========================================
+ Coverage 86.79% 86.91% +0.12%
==========================================
Files 14 14
Lines 4353 4356 +3
==========================================
+ Hits 3778 3786 +8
+ Misses 575 570 -5 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Fixes a crash in the JASP Reliability frequentist unidimensional analysis when omega computation fails (CFA Heywood/non-convergence or PFA NA) while other scale coefficients are also selected, by ensuring se (and conf for the PFA-failure path) are always populated so the scale table can render NA/blank cells instead of erroring.
Changes:
- Set
out[["se"]][["scaleOmega"]] <- NAfor omega CFA-failure and PFA-failure branches (and setconftoc(NA, NA)for PFA failure) to keep result vectors aligned. - Added a regression test + minimal dataset reproducing omega CFA failure alongside other coefficients and asserting analysis remains
"complete"and the omega row is blank/NA. - Documented the fix in
NEWS.md.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| R/reliabilityUnidimensionalFrequentist.R | Ensure omega failure branches still emit se/conf placeholders so .frequentistScaleTable() can build aligned columns. |
| tests/testthat/test-reliabilityUnidimensionalFrequentist.R | Add regression coverage for omega CFA Heywood-case failure co-occurring with other selected coefficients. |
| tests/testthat/reliabilityOmegaHeywood.csv | New near-collinear test dataset to trigger CFA omega failure deterministically. |
| NEWS.md | Changelog entry for the crash fix (Issue #4477). |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
se (and conf for PFA) never set for scaleOmega on fit failure, breaking dt$se <- unlist(...) assignment w/ mismatched row count. Fixes #4477. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
31d6f8c to
66fabac
Compare
Summary
se(andconffor the PFA path) forscaleOmegawas never set when the omega fit failed (CFA Heywood case / non-convergence, or PFANA), whileest/confalways got an entry — the resulting length mismatch crasheddt$se <- unlist(...)in.frequentistScaleTablewhenever omega was selected alongside another coefficient (e.g. Alpha).se(andconffor PFA) toNA, so the scale table renders the omega row as blank with the existing explanatory footnote instead of crashing.Fixes jasp-issues#4477.
Test plan
tests/testthat/reliabilityOmegaHeywood.csv— near-collinear items that force a CFA Heywood-case failure."complete"and the scale table shows the NA omega row.testAll()— 393 pass, 0 fail.🤖 Generated with Claude Code