Skip to content

Conversation

topepo
Copy link
Member

@topepo topepo commented Jul 16, 2021

closes #529

library(parsnip)
data(mtcars)

# engine C5.0 is only available for classification

# neither of the two specifications error
dt_reg_spec <- decision_tree() %>% 
  set_mode("regression") %>% 
  set_engine("C5.0")
#> Error: Available modes for engine C5.0 are: 'unknown', 'classification'

dt_reg_spec <- decision_tree() %>% 
  set_engine("C5.0") %>% 
  set_mode("regression")
#> Error: Available modes for engine C5.0 are: 'unknown', 'classification'

Created on 2021-07-15 by the reprex package (v2.0.0)

@topepo topepo requested a review from hfrick July 16, 2021 14:54
@hfrick
Copy link
Member

hfrick commented Jul 19, 2021

I've tried out a few combinations as additional test cases:

  • for model classes without engines we do know the possible mode(s) so it would be nice to check for that
  • when first setting the mode, then the engine, we could show available engine given the mode rather than the other way around
  • when using set_mode() without arguments, the error message is that the engine is missing (and vice versa) - this could be turned into a separate issue though?
library(parsnip)

# check mode for given model class ----------------------------------------

# can we have this error?
proportional_hazards() %>% set_mode("regression")
#> Proportional Hazards Model Specification (regression)
#> 
#> Computational engine: survival
# similar to how this errors
linear_reg() %>% set_mode("censored regression")
#> Error: Available modes for engine lm are: 'unknown', 'regression'


# check engine for given model class and mode  ----------------------------

# do we want it to send a message about available engines for the mode (rather than modes for the engine)?
dt_reg_spec <- decision_tree() %>% 
  set_mode("regression") %>% 
  set_engine("C5.0")
#> Error: Available modes for engine C5.0 are: 'unknown', 'classification'


# check for missing mode arg ----------------------------------------------

# shouldn't this error and complain about missing mode (instead of engine)?
linear_reg() %>% set_mode()
#> Error in stop_incompatible_mode(spec_modes): argument "eng" is missing, with no default

# check for missing engine arg --------------------------------------------

# shouldn't this error and complain about missing engine (instead of mode)?
linear_reg() %>% set_engine()
#> Error in eval(parse(text = text, keep.source = FALSE), envir): argument "mode" is missing, with no default

Created on 2021-07-19 by the reprex package (v2.0.0.9000)

topepo and others added 3 commits July 19, 2021 19:58
Co-authored-by: Hannah Frick <hfrick@users.noreply.github.com>
Co-authored-by: Hannah Frick <hfrick@users.noreply.github.com>
@topepo
Copy link
Member Author

topepo commented Jul 20, 2021

Fixed these issues ☝️ except:

when first setting the mode, then the engine, we could show available engine given the mode rather than the other way around

That's not trivial to figure out since they are separate function calls. I'm good with the current message.

Copy link
Member

@hfrick hfrick left a comment

Choose a reason for hiding this comment

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

Sounds good to me. I found one more thing and added a suggestion to change it

library(parsnip)

# these messages call a model type an engine
decision_tree() %>% set_mode()
#> Error: Available modes for engine decision_tree are: 'unknown', 'classification', 'regression'
proportional_hazards() %>% set_mode()
#> Error: Available modes for engine proportional_hazards are: 'unknown', 'censored regression'
proportional_hazards() %>% set_mode("regression")
#> Error: Available modes for engine proportional_hazards are: 'unknown', 'censored regression'

Created on 2021-07-20 by the reprex package (v2.0.0.9000)

topepo and others added 2 commits July 20, 2021 09:28
Co-authored-by: Hannah Frick <hfrick@users.noreply.github.com>
Co-authored-by: Hannah Frick <hfrick@users.noreply.github.com>
@topepo topepo merged commit 70d0cf8 into master Jul 20, 2021
@hfrick hfrick deleted the check-engine-mode-model branch July 20, 2021 14:28
@github-actions
Copy link

github-actions bot commented Aug 4, 2021

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 Aug 4, 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.

Error messages for incorrect mode+engine combination
2 participants