pharmaverse/admiralophtha

Bug: AFEYE does not handle nonsense data

Closed this issue · 8 comments

What happened?

If STUDYEYE or xxLAT were invalid data, AFEYE would still use values to determine STUDY or FELLOW based on the contents matching or not.

Session Information

No response

Reproducible Example

admiral_afeye_test.xlsx

Hi @barnett11 thanks for the issue.

We are using the following derivation for AFEYE:

  1. Set to "BOTH EYES" when Study Eye Selection [ADSL.STUDYEYE] is not missing, and Study Eye Laterality [AE.AELAT] is equal to "BILATERAL"
  2. Else set to "STUDY EYE" when Study Eye Selection [ADSL.STUDYEYE] is either "RIGHT" or "LEFT", and matches Study Eye Laterality [AE.AELAT] for the observation record.
  3. Else set to "STUDY EYE" when Study Eye Selection [ADSL.STUDYEYE] is "BILATERAL", and Study Eye Laterality [AE.AELAT] is not missing for the observation record
  4. Else set to "FELLOW EYE" when Study Eye Selection [ADSL.STUDYEYE] is either "RIGHT" or "LEFT", and [AE.AELAT] is not missing and does not match Study Eye Laterality [AE.AELAT] for the observation record.
  5. Else set to null
  • With regards to rows 5 and 7 of your example, then condition 3 is firing so that's why the function outputs AFEYE = "STUDY EYE".
  • With regards to rows 4 and 6 of your example, then condition 4 is firing so that's why the function outputs AFEYE = "FELLOW EYE.
  • However for row 16, while it's true that technically condition 5 should fire, I'd query whether this is a case even worth considering, since good data would have all values of STUDYEYE be in "LEFT", "RIGHT", "BILATERAL" or "" (see the outputs of derive_var_studyeye()) and so this is an example of the "garbage in, garbage out" principle where dirty inputs lead to bad outputs.

We could consider throwing a warning to check for this type of dirty data?

Thanks @manciniedoardo - I agree this is an example of "garbage in, garbage out", I wouldn't necessary agree this is what we have to live with though. I'd prefer "garbage in, nothing out" - I wouldn't want to assign records to Study Eye or Fellow Eye if we cannot tell from the data this is truly the case.

@barnett11 how would you suggest modifying the above derivation?

I would suggest only allowing the specific values of LEFT/RIGHT/BILATERAL as inputs to the derivation

@manciniedoardo @barnett11 I think it makes sense to add a check for LEFT/RIGHT/BILATERAL coming from studyeye, is it possible we may have other values coming from xxLAT though? We could add a warning if any other value comes in to prompt the user to check their data

@palmenl once you implement the change to the derivation, please can you additionally add a section in the ophtha standards vignette showcasing the exact AFEYE derivation, just for max transparency? thanks

@barnett11 this has now been merged here - feel free to take a look. Thanks @palmenl for actioning!