Stop allowing `NA` in parameter sets?
Nobody has claimed this yet.
- Dominant language
- R
- Stars
- 117
- Forks
- 36
- Avg merge
- 1h 53m
- Merged PRs (30d)
- 2
Description
parameters.list() has an inline check that rejects non-param objects before parameters_constr() is called.
This means check_list_of_param()'s NA-allowing behavior is never exercised from the parameters.list() and parameters.param() paths.
https://github.com/tidymodels/dials/blob/44ba2aa6ec6e3711bdf8cbc9949ccd3ee39fa95b/R/parameters.R#L81
Thus, can we remove that NA-allowing behaviour? We do use the constructor directly in other tidymodels packages in extract_parameter_set_dials() methods: https://github.com/search?q=org%3Atidymodels%20parameters_constr&type=code
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in R/parameters.R at the checks around lines 47-50 and 81, then trace parameters.list(), parameters.param(), parameters_constr(), and check_list_of_param(). Review the linked extract_parameter_set_dials() usages in other tidymodels packages to determine whether removing NA acceptance is safe; done means the unused behavior is removed without breaking those constructor paths.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- r
- Domain
- tooling
- Issue type
- Refactor
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 48/100