Creation of functions to set intervals after verification

[DBartlettHP]

Here is the first pass of the creation of functions noted in issue #329 .

If the functions look to do the expected tasks, I will begin on elaborate testing.

[DBartlettHP]

@billdenney Let me know if these functions are too simple or not.

Also, I want to note that I have both functions inside a single script file since they are very related.
Can I also test both of these functions within the same test script?

Edit:
Further on I noticed something within testing.

• To appropriately test assert_intervals, I would need a PKNCAdata object.
• In order to have a PKNCAdata object, I need to input intervals (in the case that dose is not given)

These two ideas seem to cause me some confusion. [Is this expected?]

[billdenney]

@billdenney Let me know if these functions are too simple or not.

Also, I want to note that I have both functions inside a single script file since they are very related. Can I also test both of these functions within the same test script?

Yes, multiple related functions should go into the same file.

Edit: Further on I noticed something within testing.

• To appropriately test assert_intervals, I would need a PKNCAdata object.
• In order to have a PKNCAdata object, I need to input intervals (in the case that dose is not given)

These two ideas seem to cause me some confusion. [Is this expected?]

The initial intervals would be a data.frame. They get tested with a PKNCAdata object that may or may not yet have the intervals added. And, the core test is related to the group column names in the PKNCAconc object. Does that make it clearer?

[DBartlettHP]

Understood, here are some further notes:

set_intervals will automatically set the intervals, after verification, in the PKNCAdata object, and return the PKNCAdata object.

set_intervals must then be used within the conditional statement if (missing(intervals)) inside the PKNCA.default function. [This caused me to move the class assignment to the front of the function, due to the verifications being used]

These changes have caused 5 tests to fail, which I am looking into:

      "The following columns in 'intervals' are not allowed"  

Where we see some tests using the interval columns:

• impute
• conc_above (i assume time_above also fails)
• aucinf
• foo

[billdenney]

Please add a note to the news file, too.

[billdenney]

Please also add a test for set_intervals().

[billdenney]

These changes have caused 5 tests to fail, which I am looking into:

      "The following columns in 'intervals' are not allowed"  

Where we see some tests using the interval columns:

• impute
• conc_above (i assume time_above also fails)
• aucinf
• foo

Can you please link to the test file lines that are failing?

For aucinf, that is probably an error where something else like aucinf.obs was intended. That test should probably be modified.

conc_above and time_above likely need some way to add them to the intervals list in some way.

impute should be added as a special case like start and end.

foo was likely an intentional test of something else. I would need to see where the error occurs to know what to do.

[DBartlettHP]

Here are the links for the failing tests:

IMPUTE
IMPUTE2
PK.CALC.ALL
PK.CALC.ALL2
PK.CALC.ALL3

Edit:

• PK.CALC.ALL: time_above and conc_above are needed for certain parameters
• PK.CALC.ALL2: seems to be covering "Unexpected intervals columns do not cause an error" which is no longer true with the new functions, I believe.
• PK.CALC.ALL3: Testing of "keep.interval.cols()" option, using "foo". (Did my functions break this?)

[DBartlettHP]

So far:

• IMPUTE and IMPUTE2: [IDEA] Perhaps it can simply be added into the allowed_columns. This seems viable due to the fact that the imputation method is used when the colnames of the intervals contains "impute"

• PK.CALC.ALL: [CORRECTED] time_above and conc_above have been added to the allowed_columns

• PK.CALC.ALL2: [CORRECTED] unexpected columns now throw an error. An error is expected in the test now.

• PK.CALC.ALL3: [IDEA] Perhaps the keep.interval_cols() option needs to also do something inside the allowed_columns within assert_intervals

[billdenney]

• IMPUTE and IMPUTE2: [IDEA] Perhaps it can simply be added into the allowed_columns. This seems viable due to the fact that the imputation method is used when the colnames of the intervals contains "impute"

Yes, "impute" can be added to the allowed_columns

• PK.CALC.ALL: [CORRECTED] time_above and conc_above have been added to the allowed_columns

Yes, time_above and conc_above can be added to the allowed_columns

• PK.CALC.ALL2: [CORRECTED] unexpected columns now throw an error. An error is expected in the test now.

That's the right change.

• PK.CALC.ALL3: [IDEA] Perhaps the keep.interval_cols() option needs to also do something inside the allowed_columns within assert_intervals

keep_interval_cols is an important exception. Yes, keep_interval_cols should prevent errors in assert_intervals().