Skip to content

Deprecate default argument #241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Jul 12, 2022
Merged

Deprecate default argument #241

merged 18 commits into from
Jul 12, 2022

Conversation

hfrick
Copy link
Member

@hfrick hfrick commented Jun 10, 2022

This PR removes the default argument to the constructors new_quant_param() and new_qual_param(). Closes #229 and closes #153.

The default value was used in a single place: when generating a sequence of parameter values of length 1 via value_seq(n = 1). This PR removes the special behaviour for n = 1 in value_seq() and removes all the default values for the exported parameter functions like Laplace().

The revdep checks came back clean.

hfrick added 3 commits June 10, 2022 20:00
because the constructor is called in a lot of places which do not actually make use of the `default` arg
@hfrick hfrick marked this pull request as ready for review July 8, 2022 16:44
@hfrick hfrick requested review from DavisVaughan and topepo July 8, 2022 16:45
@hfrick
Copy link
Member Author

hfrick commented Jul 8, 2022

@DavisVaughan I've addressed the comments from our review session 🙌

@topepo Max, could you also have a look at this since the PR removes the default values also for the exported parameter objects?

Copy link
Member

@DavisVaughan DavisVaughan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I think this makes things a lot simpler and more consistent overall

@hfrick hfrick merged commit 5c055ab into main Jul 12, 2022
@hfrick hfrick deleted the deprecate-default-arg branch July 12, 2022 09:50
@hfrick hfrick mentioned this pull request Jul 12, 2022
@github-actions
Copy link

This pull request 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.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

default values should be within range or values default values for tuning parameters should be within range
3 participants