Skip to content

Check valid spec mode #470

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 9 commits into from
May 3, 2021
Merged

Conversation

jtlandis
Copy link
Contributor

Hello,

This pull request is to fix the small bug found in #467 . This pull request will prevent users from adding an incompatible mode to a model_spec object via set_mode.

The implementation was simple:

  1. refactor section of new_model_spec that checks class and mode compatibility into a separate function check_spec_mode_val
  2. replace code in new_model_spec with new function
  3. add new function to set_mode

I also added about 47 additional tests in the file tests/testthat/test_spec_and_modes.R. Some of these may be redundant as they simply check that compatible modes can be added to a given model_spec. The novel part of the test that script includes expect_error for when adding an improper mode type.

@jtlandis
Copy link
Contributor Author

I will make these changes and redo the PR 👍

Copy link
Member

@juliasilge juliasilge left a comment

Choose a reason for hiding this comment

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

Thank you so much @jtlandis! 🙌

@juliasilge juliasilge merged commit b1cb0bd into tidymodels:master May 3, 2021
@jtlandis jtlandis deleted the check_valid_spec_mode branch May 3, 2021 22:44
@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 May 18, 2021
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.

3 participants