Skip to content

Ap identification - #1854

Open
SFJohnson24 wants to merge 14 commits into
mainfrom
ap_identification
Open

SFJohnson24 wants to merge 14 commits into
mainfrom
ap_identification

Conversation

@SFJohnson24

Copy link
Copy Markdown
Collaborator

This PR updates logic for identifying AP class which used to seek out the parent dataset then ID based on that. This PR still uses that logic, if it fails, it attempts to use the AP suffix--testing it in the case of both standard and custom domains attached to AP--.

@pendingintent pendingintent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AP-- will not have an associated domain other than APRELSUB. MH will not have a relationship to APMH. All AP domains are only to record details for a person associated with a USUBJID. There is no parent/child relationship with AP--. The suffix only indicates the type of data collected in the dataset. In this case, this is medical history for the associated person. The APID in APMH will join to APRELSUB.APID and the APRELSUB.USUBJID will indicate to which subject the person is associated.

@SFJohnson24

SFJohnson24 commented Sep 10, 2026 •

Copy link
Copy Markdown
Collaborator Author

I removed the logic for AP that I preserved, that was built on a false premise.
I also updated ap_suffix for APRELSUB/APRELSPEC as it would return "" for ap_suffix. In returning this, the the suffix is used to ID the dataset as RELSUB/RELSPEC and thus relationship. It would not impact wildcard replacement as there are not wildcard variables in either dataset from what I can tell from library

I updated tests as the ap_suffix had several failing tests where there was no metadata on the dataset--meaning a dataset was submitted but completely empty. It no longer returns "" and instead would error out given the total lack of metadata. I would expect this given the blank data which would force a rule skip for that dataset and rule

@SFJohnson24

Copy link
Copy Markdown
Collaborator Author

AP-Domains.xlsx
I tested CORE-000376 which used the custom_domain logic. There was another bug there with AP that I resolved. The attached data should be positive data and return no bugs running this rule.

@pendingintent pendingintent left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AP-Domains-negative.xlsx
CORE-Report-2026-09-15T10-13-10.xlsx

I have executed the API-Applicable rules against the datasets in AP-Domains-negative.xlsx.

python core.py validate -s sdtmig -v 3-4 -r CORE-000181 -r CORE-000233 -r CORE-000234 -r CORE-000235 -r CORE-000778 -r CORE-000180 -r CORE-000510 -r CORE-000201 -r CORE-000376 -dp /Users/dmoreland/Downloads/AP-Domains.xlsx

I am seeing one issue. CORE-000778 is showing a false positive - Associated Persons non-supplemental qualifier dataset associated with a split dataset does not have a dataset name with a length greater than 4 and less than, or equal to, 6. There is no split dataset included in the test data.

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated schema has not been merged with markdown descriptions. Please run the "Merge Schema with Markdown Descriptions" workflow to update the merged schema files.

@SFJohnson24

Copy link
Copy Markdown
Collaborator Author

note: the test suite failure is expected given the changes in scope (rules involving split and supp are failing)

This branch was successfully deployed

1 active deployment
DEV — 4dbd7535 Deployed Oct 1, 2026 by SFJohnson24 via deploy_rule_tester #901
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

APxx datasets giving false positives, even when -v 3-4 is given.

2 participants