medians for ordinals, no conversion for quantiles#471
medians for ordinals, no conversion for quantiles#471JohnnyDoorn wants to merge 2 commits intojasp-stats:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds median and IQR support for ordinal variables in the JASP Descriptives module, and simplifies the quantile computation for ordinals by removing the as.numeric() conversion. Previously, ordinal variables were converted to numeric codes before computing quantiles, and then labels were looked up afterward. Now, quantile() is called directly on the ordered factors (which only supports types 1 and 3), with a helper function .ordinalQuantileType() that falls back to type 3 for unsupported types.
Changes:
- Added ordinal median (using
quantile()on ordered factors with type fallback) and ordinal IQR (usingstats::IQR()) to the descriptives table, and changed the Median column type from"number"to"mixed"to support both numeric and string outputs. - Simplified quartile, equal-group, and percentile computations for ordinals by calling
quantile()directly on ordered factors instead of converting to numeric and then mapping back to labels. - Added
.ordinalQuantileType()helper function that restricts quantile types to 1 or 3 (the only types R'squantile()supports for ordered factors), defaulting to type 3 for unsupported types.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } else if (columnType == "ordinal") { | ||
| ordinalType <- .ordinalQuantileType(as.integer(options[["quantilesType"]])) | ||
| if (options$median) { | ||
| medianEst <- quantile(na.omitted, 0.5, names = FALSE, type = ordinalType) | ||
| resultsCol[["Median"]] <- toMixedCol(medianEst) | ||
| } | ||
| if (options$iqr) | ||
| resultsCol[["IQR"]] <- stats::IQR(na.omitted, type = ordinalType) |
There was a problem hiding this comment.
The new ordinal median and IQR code paths lack test coverage. The existing test at line 546 of test-descriptives.R covers ordinal quartiles/percentiles but does not enable options$median or options$iqr. Consider adding a test that enables median = TRUE and iqr = TRUE for ordinal variables to verify both the label output for medians and the numeric output for IQR. This is especially important to verify the silent type fallback behavior (e.g., when user selects type 7 but ordinals fall back to type 3).
| # R's quantile() on ordered factors only supports types 1 and 3. | ||
| # Fall back to type 3 when the user-selected type is incompatible. | ||
| .ordinalQuantileType <- function(type) { | ||
| if (type %in% c(1L, 3L)) type else 3L | ||
| } |
There was a problem hiding this comment.
When the user selects a quantile type other than 1 or 3 (e.g., type 7, the R default), .ordinalQuantileType silently falls back to type 3 for ordinal variables. This means the user's chosen setting is overridden without any notification. Consider adding a footnote to the results table (similar to the existing footnote pattern at lines 520–543) to inform the user that the quantile type was adjusted for ordinal variables, e.g., "For ordinal variables, quantile type 3 was used because the selected type is not supported."
Medians for ordinals, in the same way we provide quantiles.
Different approach for the quantile types, which omits the 'as.numeric()' so the R-functions are using the columns as provided (for ordinals the type now defaults to 3 if any other than 1 or 3 is selected).
Can still include a footnote about the mean?