[jaspFactor] Enable ordinal variable support in Exploratory Factor Analysis by...

[sisyphus-jasp]

Summary

Fixes: jasp-stats/jasp-issues#4005

Root cause

• In R/principalcomponentanalysis.R, .pcaAndEfaCoerceToNumeric() used variableType %in% c("ordinal", "nominal") || ....
• When variableType was NULL, first term became logical(0), which could propagate to an invalid/NA condition in if (conversionIntroducedNA && isCategorical) and error at runtime in fallback paths.

What changed and why

• Added explicit guard:
  • isTypeCategorical <- !is.null(variableType) && variableType %in% c("ordinal", "nominal")
  • isCategorical <- isTypeCategorical || is.factor(x) || is.ordered(x)
• This preserves existing behavior for typed/factor/ordered inputs while making missing variables.types fallback safe.

Verification

• Full suite via fallback harness: agentTestAll() => FAIL: 0 | WARN: 0 | SKIP: 13 | PASS: 131.
• Reproduced Stage 1 scenario before fix (x <- c("A","1"), variableType = NULL) previously errored; after fix no error.
• Re-checked additional Stage 1 edge input (factor(c("low","high")), variableType = NULL) no error.

Caveats / reviewer checks

• In this container fallback harness, plain testAll() path lacked runAnalysis on search path; used documented fallback agentTestAll() for valid full-suite verification.

Implementation Plan

Root cause

In R/principalcomponentanalysis.R, .pcaAndEfaCoerceToNumeric() computes:
isCategorical <- variableType %in% c("ordinal", "nominal") || is.factor(x) || is.ordered(x).
When variableType is NULL, variableType %in% ... is logical(0). With non-factor input, isCategorical can become NA; then if (conversionIntroducedNA && isCategorical) errors (missing value where TRUE/FALSE needed). This breaks fallback path when options[["variables.types"]] is absent/incomplete.

Proposed changes

• File: R/principalcomponentanalysis.R
• Function: .pcaAndEfaCoerceToNumeric()
• Change:
  • Add explicit guard: isTypeCategorical <- !is.null(variableType) && variableType %in% c("ordinal", "nominal")
  • Recompute: isCategorical <- isTypeCategorical || is.factor(x) || is.ordered(x)
• Keep all other behavior unchanged.

Expected test impact

• Existing tests should pass; behavior only changes for fallback NULL type edge cases.
• Reproduction case should stop erroring and return numeric/factor-coded values as intended.

REVIEW ITERATION 2 of 3

A code review agent examined your previous fix and found issues.
Your previous changes are still in the workspace.

REVIEWER FEEDBACK:
The fix addresses the reported root cause for labeled ordinal data in the common path, but .pcaAndEfaCoerceToNumeric() has a correctness issue:

• isCategorical <- variableType %in% c("ordinal", "nominal") || ... is unsafe when variableType is NULL (your intended fallback case). In R, NULL %in% ... yields logical(0), and logical(0) || ... triggers a length-0 logical error.

This means the newly introduced fallback behavior can fail at runtime instead of gracefully handling missing options[["variables.types"]].

Actionable fix:

• Guard the type check explicitly, e.g. isTypeCategorical <- !is.null(variableType) && variableType %in% c("ordinal", "nominal")
• Then compute isCategorical <- isTypeCategorical || is.factor(x) || is.ordered(x).

After that change, the patch should be in good shape for approval.

SPECIFIC ISSUES:

• R/principalcomponentanalysis.R: .pcaAndEfaCoerceToNumeric() can error when variableType is NULL due to logical(0) || ... in isCategorical computation, breaking the intended fallback path.

Address the reviewer's feedback. You may skip Stage 1 (REPRODUCE).

ORIGINAL TASK:
Enable ordinal variable support in Exploratory Factor Analysis by computing polychoric correlations, reusing existing CFA implementation patterns

Test Results

Test Run	Result
Baseline (pre-fix)	[ FAIL 0 | WARN 0 | SKIP 13 | PASS 132 ]
Post-fix	[ FAIL 0 | WARN 0 | SKIP 13 | PASS 132 ]
Upstream CI	e71f5ad -- CI: failing

Automated Code Review

Approved after 2 review iteration(s).