Skip to content

Conversation

@brookslogan
Copy link

@brookslogan brookslogan commented Mar 25, 2025

  • Addresses quantile_pred() and vctrs missingness treatment don't play well #281, where vctrs/tidyverse operations adding conceptual NAs to a quantile_pred object resulted in many operations failures.
  • Changes vec_detect_missing() to treat the solely following as missing: a quantile_pred entry with NA quantile values at all quantile levels.
  • Fixes vec_detect_complete() to conform with expectations; an NA value at any quantile level means that that prediction is incomplete.
  • Fixes == to conform to expectations with respect to NAs.
  • To accomplish the above changes, moves from a list-of-rows representation to a matrix representation. The matrix is stored inside of a rcrd to avoid some vctrs operations from misbehaving.
  • Adds ptype2 and cast implementations to make it so that vec_c-ing an integer-matrix-based quantile_pred and a double-matrix-based quantile_pred is still possible.
  • Provides some more helpful messaging when combining predictions with quantile level sets that look like they are identical but differ in the undisplayed digits. This type of issue has occurred when levels that are multiples of 0.05 have been requested in the past. This error is still not the friendliest behavior, but is at least a step in the right direction.

@brookslogan brookslogan force-pushed the quantile_pred-NA-handling branch from f70d198 to 29dd62c Compare March 25, 2025 01:06
@brookslogan
Copy link
Author

brookslogan commented Mar 25, 2025

Sorry, just fixed up a missing test. Think this should be good to review now.

@brookslogan
Copy link
Author

Regarding check issues:

  • it looks like tibble( might need to be tibble::tibble(
  • dplyr isn't in Suggests:; dplyr::full_join could become merge
    • printing plain data.frames containing quantile_pred columns (e.g., out of merge) doesn't work; looks like digits = NULL is explicitly being passed/forwarded to format.quantile_pred.

Working to address these.

@dajmcdon dajmcdon mentioned this pull request Aug 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant