check_type in derive_vars_merged() not working

[kaz462]

Background Information

check_type in derive_vars_merged() is not working as expected when order is NULL
example: check_type = warning but an error is returned

adsl <- tribble(
  ~USUBJID, ~SEX, ~COUNTRY,
  "ST42-1", "F",  "AUT"
)

advs <- tribble(
  ~USUBJID, ~PARAMCD, ~AVISIT,    ~AVISITN, ~AVAL,
  "ST42-1", "WEIGHT", "BASELINE",        0,    66,
  "ST42-1", "WEIGHT", "WEEK 2",          1,    68
)

derive_vars_merged(
  adsl,
  dataset_add = advs,
  by_vars = exprs(USUBJID),
  new_vars = exprs(WEIGHT = AVAL),
  check_type = "warning"
)

error message:

Error in `signal_duplicate_records()`:
! Dataset `dataset_add` contains duplicate records with respect to `USUBJID`
Run `get_duplicates_dataset()` to access the duplicate records
Run `rlang::last_trace()` to see where the error occurred.

Definition of Done

add cnd_type = check_type when calling the following signal_duplicate_records

admiral/R/derive_merged.R

Lines 375 to 379 in 16671ee

	signal_duplicate_records(
	add_data,
	by_vars = by_vars_right,
	msg = duplicate_msg
	)

[Lina2689]

Hi @kaz462 , after adding cnd_type = check_type to signal_duplicate_records call, its working and throwing warning message when check_type = "warning" . Should I update this and create PR?

[bundfussr]

Hi @kaz462 , hi @Lina2689 , it is intended that an error is issued regardless of check_type if order is not specified. derive_vars_merged() should add variables but it shouldn't change the number of records. If there is more than one record per by group in dataset_add (after filtering), the left_join() in line 419, would add records to the input dataset. If order is specified, this can not happen because the filter_extreme() call in line 361 ensures that there is not more than one records per by group.

So please do not change the code.

[kaz462]

@bundfussr @Lina2689 Thanks both for looking into it. Should we add documentation regarding the behavior of the check_type argument? If the order argument is not specified, the check_type argument is ignored.

[bms63]

I think we should update the documentation for the arguments. @Lina2689 do you mind doing this?

[Lina2689]

Sure @bms63. and thanks for clarifying the issue.

[Lina2689]

@bms63 , do I need to update/modify both vignette and information in the function header documentation?

[bms63]

@bms63 , do I need to update/modify both vignette and information in the function header documentation?

Yes please! Where ever the arguments are discussed it should be updated. If not discussed in the https://pharmaverse.github.io/admiral/articles/generic.html then I think it warrants an inclusion