NULL Elements Patch

[rymarinelli]

closes #555

[kyleam]

Due to NULL values being able to elements in a list, some problematic behavior arises.

I'm uncertain about whether it's problematic. The user sets it to an invalid value, and then when they submit the model, they see

Error in check_bbi_args(.bbi_args) :
There are 1 errors in check_bbi_args(): `` passed for arg `threads`. Expected 'numeric' type.

The messaging could be clearer, but that seems fair enough. So, my concern is that this PR is adding a fairly complicated function to solve a non-problem. Am I missing something?

Also, is your intent for this PR to supersede gh-554 given it takes the code suggestion from there?

[rymarinelli]

Due to NULL values being able to elements in a list, some problematic behavior arises.

I'm uncertain about whether it's problematic. The user sets it to an invalid value, and then when they submit the model, they see

Error in check_bbi_args(.bbi_args) :
There are 1 errors in check_bbi_args(): `` passed for arg `threads`. Expected 'numeric' type.

The messaging could be clearer, but that seems fair enough. So, my concern is that this PR is adding a fairly complicated function to solve a non-problem. Am I missing something?

Also, is your intent for this PR to supersede gh-554 given it takes the code suggestion from there?

Yes, I think I need to resolve this first before moving back to #554. I agree that the error that raises is minor, but the associated yaml could be problematic with some of the models. For instance, the yaml could reach this state.

model_type: nonmem
description: original test-workflow-bbi model
bbi_args:
  parallel: no
  threads: ~

@seth127 What are your thoughts on this? Do you think this is worth the complexity? Also, do you have any suggestions on the organization of the two highly related PRs?

[kyleam]

the associated yaml could be problematic with some of the models. For instance, the yaml could reach this state.

Can you flesh out for me a bit more what the problem is? Is it something beyond "the user attaches invalid argument type to a model, and the submission aborts when it checks the type of those arguments"?

[rymarinelli]

I think the primary concern is with new_model. If you passed in NULL values into .bbi_args it could reach undesired states.

For instance, you might have black spaces associated with values.

── BBI Args ───────────────────────────────────────────────────────────────────────────────────────────────────────────
• parallel: FALSE
• threads: 

The other concern was with the yaml, and if that could cause any issues with how it is being written out.

A secondary concern is that some handling might not be working if passed for arg `threads`. Expected 'numeric' type. is being reached. It is my understanding the bbi_args should not be NULL and should be promptly removed in the logic if ever set to NULL. The goal of writing the function was to try and write some more generalized handling that could be applied to wherever bbi_args is touched to ensure that no NULL values are passed.

[seth127]

I think this PR can be closed without merging, given our conclusion in this comment

@kyleam do you agree?

[kyleam]

@seth127 Thanks for summarizing our discussion in the gh-555 comment.

@kyleam do you agree?

Yes.