apply suggestions from review

* edits to `validate_has_calibration()`:
     * refer to "The workflow" rather than `caller_arg()`
     * Warn rather than error on unneeded calibration set
     * All arguments on one line in function signature
* comment on expectation re: predictions
* doc tweaks

R/fit.R

@@ -226,19 +226,18 @@ validate_has_model <- function(x, ..., call = caller_env()) {
   invisible(x)
 }
 
-validate_has_calibration <- function(x, calibration,
-                                     x_arg = caller_arg(x), call = caller_env()) {
+validate_has_calibration <- function(x, calibration, call = caller_env()) {
   if (.should_inner_split(x) && is.null(calibration)) {
     cli::cli_abort(
-      "{.arg {x_arg}} requires a {.arg calibration} set to train but none
+      "The workflow requires a {.arg calibration} set to train but none
        was supplied.",
       call = call
     )
   }
 
   if (!.should_inner_split(x) && !is.null(calibration)) {
-    cli::cli_abort(
-      "{.arg {x_arg}} does not require a {.arg calibration} set to train
+    cli::cli_warn(
+      "The workflow does not require a {.arg calibration} set to train
        but one was supplied.",
       call = call
     )

R/post-action-tailor.R

@@ -27,7 +27,7 @@
 #' preprocessor and model. However, in the case where a postprocessor must be
 #' trained as well, allotting all of the available data to the `data` argument
 #' to train the preprocessor and model would leave no data
-#' left to train the postprocessor with---if that were the case, workflows
+#' to train the postprocessor with---if that were the case, workflows
 #' would need to `predict()` from the preprocessor and model on the same `data`
 #' that they were trained on, with the postprocessor then training on those
 #' predictions. Predictions on data that a model was trained on likely follow

tests/testthat/_snaps/fit.md

@@ -35,18 +35,18 @@
 # fit.workflow confirms compatibility of object and calibration
 
     Code
-      fit(workflow, mtcars, calibration = mtcars)
+      res <- fit(workflow, mtcars, calibration = mtcars)
     Condition
-      Error in `fit()`:
-      ! `object` does not require a `calibration` set to train but one was supplied.
+      Warning in `fit()`:
+      The workflow does not require a `calibration` set to train but one was supplied.
 
 ---
 
     Code
       fit(workflow, mtcars)
     Condition
       Error in `fit()`:
-      ! `object` requires a `calibration` set to train but none was supplied.
+      ! The workflow requires a `calibration` set to train but none was supplied.
 
 # can `predict()` from workflow fit from individual pieces
 

tests/testthat/test-fit.R

@@ -95,9 +95,9 @@ test_that("fit.workflow confirms compatibility of object and calibration", {
   workflow <- add_formula(workflow, mpg ~ cyl)
   workflow <- add_model(workflow, mod)
 
-  expect_snapshot(error = TRUE, {
-    fit(workflow, mtcars, calibration = mtcars)
-  })
+  expect_snapshot(
+    res <- fit(workflow, mtcars, calibration = mtcars)
+  )
 
   tailor <- tailor::tailor()
   tailor <- tailor::adjust_numeric_calibration(tailor)

tests/testthat/test-post-action-tailor.R

@@ -137,5 +137,7 @@ test_that("postprocessor fit aligns with manually fitted version (with calibrati
   wflow_post_preds <- predict(wf_post_fit, dat_calibration)
 
   expect_equal(wflow_manual_preds[".pred"], wflow_post_preds)
+
+  # okay if some predictions are the same, but we wouldn't expect all of them to be
   expect_false(all(wflow_simple_preds[".pred"] == wflow_manual_preds[".pred"]))
 })