Skip to content

Conversation

@atrigila
Copy link
Contributor

validatefomcomponents always required a required comparison (variable, reference, target) even when a formula and make_contrasts_str were supplied. This was because the internal TSV structure expected variable, reference, and target column.

The changes introduced here aim to:

  1. Remove automatic blocking inference from formula
  2. The YAML contrast must specify blocking_factors if needed; otherwise blocking is NA.
  3. variable, reference, and target are validated only when comparison is provided, not when formula is used.
  4. Add a test case for YAML with testthat.

@pinin4fjords
Copy link
Owner

Maybe it would be simpler to initialise the frame and then set the values? Also think we can be a little less verbose.

# Parse YAML contrasts into a data frame
contrasts <- do.call(rbind, lapply(contrasts_yaml$contrasts, function(x) {
  stopifnot(!is.null(x$id))  # Require that each contrast has an 'id'

  # --- Start with a data frame with all fields set to NA ---
  row <- data.frame(
    id = x$id,
    variable = NA, reference = NA, target = NA,
    blocking = NA,
    formula = NA,
    make_contrasts_str = NA,
    stringsAsFactors = FALSE
  )

  if (!is.null(x$comparison)) {
    # --- 'comparison'-based contrast ---
    if (!is.null(x$formula) || !is.null(x$make_contrasts_str)) {
      stop(sprintf("Contrast id '%s' with 'comparison' must not have 'formula' or 'make_contrasts_str'.", x$id))
    }

    fields <- setNames(rep(NA, 3), c("variable", "reference", "target"))
    fields[names(fields)[seq_along(x$comparison)]] <- x$comparison

    if (any(is.na(fields)) || any(fields == "")) {
      stop(sprintf("Contrast id '%s' must provide non-empty 'variable', 'reference', and 'target' fields.", x$id))
    }

    row$variable <- fields["variable"]
    row$reference <- fields["reference"]
    row$target <- fields["target"]

    if (!is.null(x$blocking_factors)) {
      row$blocking <- paste(x$blocking_factors, collapse = ";")
    }

  } else if (!is.null(x$formula)) {
    # --- 'formula'-based contrast ---
    if (is.null(x$make_contrasts_str) || !is.null(x$blocking_factors)) {
      stop(sprintf("Contrast id '%s' with 'formula' must have 'make_contrasts_str' and no 'blocking_factors'.", x$id))
    }

    row$formula <- x$formula
    row$make_contrasts_str <- x$make_contrasts_str

  } else {
    stop(sprintf("Contrast id '%s' must provide either 'comparison' or 'formula' + 'make_contrasts_str'.", x$id))
  }

  row
}))

@pinin4fjords
Copy link
Owner

This is still draft. Good to go?

@atrigila atrigila marked this pull request as ready for review April 30, 2025 12:40
@atrigila
Copy link
Contributor Author

Yes, thank you!

Co-authored-by: Jonathan Manning <[email protected]>
@pinin4fjords pinin4fjords merged commit 71e3889 into pinin4fjords:develop May 2, 2025
1 check passed
@pinin4fjords
Copy link
Owner

bioconda/bioconda-recipes#55859

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.

2 participants