Skip to content
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

pir_run's model_select_params may be a model_select_params (instead of a list of 1 element which is a model_select_params) #36

Closed
richelbilderbeek opened this issue Jan 21, 2019 · 3 comments
Assignees
Labels
high priority Do this first
Projects

Comments

@richelbilderbeek
Copy link
Owner

richelbilderbeek commented Jan 21, 2019

Currently, pir_run's model_select_params argument assumes a list of 1 or more elements:

model_select_params <- list(
  create_gen_model_select_param(
    alignment_params = alignment_params,
    tree_prior = create_bd_tree_prior()
  )
)
pir_run(
  model_select_params = model_select_params,
  ...
)

Creating a list is awkward for the user. pir_run must up the params to a list, allowing this interface:

# This should work
model_select_param <- create_gen_model_select_param(
  alignment_params = alignment_params,
  tree_prior = create_bd_tree_prior()
)
pir_run(
  model_select_params = model_select_param,
  ...
)

Doing this is simple: in pir_run if is_model_select_param(model_select_params) == TRUE, model_select_params is not a list and must be made so. Something like this:

if (is_model_select_param(model_select_params) == TRUE) {
  model_select_params <- list(model_select_params)
}

Write a test that the listing marked with # This should work works fine, using expect_silent.

@richelbilderbeek
Copy link
Owner Author

richelbilderbeek commented Jan 21, 2019

I think @Giappo will like this one...

If I have not been clear enough, go ahead and hit me with some questions 👍

@richelbilderbeek richelbilderbeek added medium priority Do this after the high priority Issues high priority Do this first and removed medium priority Do this after the high priority Issues labels Jan 23, 2019
@richelbilderbeek richelbilderbeek moved this from To do to In progress in v1.2 Jan 23, 2019
@richelbilderbeek richelbilderbeek moved this from In progress to Maybe done in v1.2 Jan 23, 2019
@Giappo Giappo moved this from Maybe done to To do in v1.2 Jan 23, 2019
@Giappo Giappo moved this from To do to In progress in v1.2 Jan 23, 2019
@Giappo Giappo moved this from In progress to Maybe done in v1.2 Jan 23, 2019
@richelbilderbeek
Copy link
Owner Author

Great, I'll stress-test this...

@richelbilderbeek richelbilderbeek moved this from Maybe done to In progress in v1.2 Jan 24, 2019
@richelbilderbeek richelbilderbeek self-assigned this Jan 24, 2019
richelbilderbeek pushed a commit that referenced this issue Jan 24, 2019
@richelbilderbeek richelbilderbeek moved this from In progress to Maybe done in v1.2 Jan 24, 2019
@richelbilderbeek
Copy link
Owner Author

Passed the stress-testing, closing this Issue

v1.2 automation moved this from Maybe done to Done Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high priority Do this first
Projects
v1.2
  
Done
Development

No branches or pull requests

2 participants