[GEN-867] Add validation rule to check if INT_DOD >= INT_CONTACT

[danlu1]

Problem:

Lack of validation check on whether INT_DOD >= INT_CONTACT

Solution:

Add the validation check to compare INT_DOD and INT_CONTACT when both of them exist and not NAs.

Testing:
Unit test has been added.

[BryanFauble]

Try to limit the amount of in-line comments you add. In a lot of cases it has the unintended affect of reducing readability. The code you wrote does well at telling us what the code is doing. That should be the source of truth.

[danlu1]

Sure. Will do.

[BryanFauble]

This inline comment is misleading. You are only checking for False.

[danlu1]

Hmmm let me rephrase this.

[BryanFauble]

Is the type here a list of pd.Index? The length check below suggests it's an iterator of values.

[BryanFauble]

Might also just be the pandas specific way of doing things that is not super intuitive

[danlu1]

The type is pandas.core.indexes.range.RangeIndex. We can either use len or size.

[BryanFauble]

Nit:
errors, _
As warnings is not used

[danlu1]

This is adapted from

Genie/genie_registry/clinical.py

Lines 475 to 535 in 3f31802

	def _validate_oncotree_code_mapping(
	self: "Clinical", clinicaldf: pd.DataFrame, oncotree_mapping: pd.DataFrame
	) -> pd.Index:
	"""Checks that the oncotree codes in the input clinical
	data is a valid oncotree code from the official oncotree site
	Args:
	clinicaldf (pd.DataFrame): clinical input data to validate
	oncotree_mapping (pd.DataFrame): table of official oncotree
	mappings
	Returns:
	pd.Index: row indices of unmapped oncotree codes in the
	input clinical data
	"""
	# Make oncotree codes uppercase (SpCC/SPCC)
	clinicaldf["ONCOTREE_CODE"] = (
	clinicaldf["ONCOTREE_CODE"].astype(str).str.upper()
	)
	unmapped_oncotrees = clinicaldf[
	(clinicaldf["ONCOTREE_CODE"] != "UNKNOWN")
	& ~(clinicaldf["ONCOTREE_CODE"].isin(oncotree_mapping["ONCOTREE_CODE"]))
	]
	return unmapped_oncotrees.index
	def _validate_oncotree_code_mapping_message(
	self: "Clinical",
	clinicaldf: pd.DataFrame,
	unmapped_oncotree_indices: pd.DataFrame,
	) -> Tuple[str, str]:
	"""This function returns the error and warning messages
	if the input clinical data has row indices with unmapped
	oncotree codes
	Args:
	clinicaldf (pd.DataFrame): input clinical data
	unmapped_oncotree_indices (pd.DataFrame): row indices of the
	input clinical data with unmapped oncotree codes
	Returns:
	Tuple[str, str]: error message that tells you how many
	samples AND the unique unmapped oncotree codes that your
	input clinical data has
	"""
	errors = ""
	warnings = ""
	if len(unmapped_oncotree_indices) > 0:
	# sort the unique unmapped oncotree codes
	unmapped_oncotree_codes = sorted(
	set(clinicaldf.loc[unmapped_oncotree_indices]["ONCOTREE_CODE"])
	)
	errors = (
	"Sample Clinical File: Please double check that all your "
	"ONCOTREE CODES exist in the mapping. You have {} samples "
	"that don't map. These are the codes that "
	"don't map: {}\n".format(
	len(unmapped_oncotree_indices), ",".join(unmapped_oncotree_codes)
	)
	)
	return errors, warnings

and we might need to output warnings in the future.

[BryanFauble]

A few formatting suggestions, otherwise the business logic looks good!

[rxu17]

LGTM! Just one comment

[rxu17]

This part of the message is bit misleading because it implies that the INT_DOD field is the issue when it could be INT_CONTACT. You could rephrase this part of the error message to be like:
The row number(s) this occurs in are: {invalid_year_death_indices.tolist()}. Please correct.

I think this is happening for the YEAR_DEATH AND YEAR_CONTACT validation rule function too.

[thomasyu888]

^ Please fix the above in this PR as well. Thanks!

[thomasyu888]

🔥 Fantastic work, and great code reviews! Thanks team - will pre-approve.

[sonarqubecloud]

Quality Gate passed

Issues
8 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud