Should varying_args.step() actually remove NULL arguments?

[DavisVaughan]

Currently, varying_args.step() removes any argument where the value is NULL. I'm not sure this is the right behavior. Consider this example:

library(parsnip)
library(recipes)

x <- recipe(Species ~ ., data = iris) %>%
  step_bs(Sepal.Length, degree = varying())

varying_args(x)
#> # A tibble: 2 x 4
#>   name    varying id      type 
#>   <chr>   <lgl>   <chr>   <chr>
#> 1 degree  TRUE    step_bs step 
#> 2 options FALSE   step_bs step

^{Created on 2019-01-24 by the reprex package (v0.2.1.9000)}

In this case deg_free is a parameter that could validly have been tuned over, but it was not shown here because its default value is NULL (and nulls are removed). On the other hand, options has a default of list(), so it shows up here.

I think we should not remove any NULL values, so we can print the full set of potential tuning parameters.

[DavisVaughan]

This is interesting actually because for steps such as step_knnimpute(), the potential varying arguments such as K default to NULL, and you would want to show those. BUT other arguments specific to that step such as columns also default to NULL, but these aren't really potential varying args, and should not really be set by the user (they are set at prep time automatically). This second case is probably the original motivation for why NULL values are removed.

I still think it would be better to show everything, and deal with the false positives. Alternatively, a hand curated list of step specific arguments that are not varying could be maintained (similar but more fine grained to the one that holds global args like "role") and those would be excluded.

(okay, maybe K is a bad example because it is deprecated, but the argument still holds with deg_free from above, which defaults to null)

[DavisVaughan]

Okay, so it seems that in recipes, NULL is almost always reserved for arguments that have been deprecated or are set at prep() time.

There are a few exceptions (step_bs() with deg_free happens to be one), and I have gone through and collected the others, along with suggested tweaks to recipes that can be made to not use NULL.

• step_bs - deg_free - maybe use NA in the step, but pass NULL through when needed

• step_geodist - lon / lat / ref_lon / ref_lat - all required! Leave them as blank arguments

• step_pls - outcome - required! Leave blank

• step_profile - profile - required! Leave blank

• step_sample - size - Maybe use NA instead?

• step_window - names - Maybe use NA instead?

If we fix these over in recipes, then filtering out NULL values is valid, and seems like the right thing to do. That way, all possible tunable recipe steps will be shown using varying_args()

[github-actions]

This issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.