Skip to content

Conversation

@SFJohnson24
Copy link
Collaborator

@SFJohnson24 SFJohnson24 commented Dec 18, 2025

This PR adds logic for AP-- domains, mainly SQAP as well as AP--. The logic to get the associated persons, replace USUBJID for APID, and grab the remaining domain variables, then merge. It also adds a test suite for base domains, SQAP, SUPP--, as well as AP-- domains.

to test: run cg0013 on the AP datasets in dev folder.

This pull request refactors how variable metadata is retrieved and handled for SDTM domains, especially for special cases like SUPP, SQ, and AP (Associated Persons) domains. It removes hardcoded logic for required variables, centralizes domain handling, and improves support for AP domains by dynamically merging Associated Persons variables when needed. Additionally, it updates the logic for determining variable permissibility and aligns unit tests with the new approach.

Core logic and metadata handling improvements:

  • Refactored get_variables_metadata_from_standard and get_variables_metadata_from_standard_model in sdtm_utilities.py to handle AP (Associated Persons), SUPP, and SQ domains more robustly by dynamically merging AP variables and removing hardcoded checks for required variables. This includes sorting and filtering logic for identifiers and updating how wildcards are replaced in variable names. [1] [2] [3] [4] [5] [6]

  • Updated base_dataset_builder.py and base_operation.py to consistently use unsplit_name for domain resolution, removing special-case logic for APREL and SUPP domains. This centralizes domain handling and ensures consistency across the codebase. [1] [2]

Permissibility and required variables logic:

  • Removed the use of hardcoded REQUIRED_MODEL_VARIABLES and SEQ_VARIABLE from permissibility.py and related logic in base_operation.py. The permissibility for variables is now determined directly from metadata, improving maintainability and flexibility. [1] [2] [3]

Testing and documentation:

  • Updated unit tests for permissible and required variables to match the new dynamic approach, ensuring that expected variables reflect changes in how required and permissible variables are determined. [1] [2]

Documentation and comments:

  • Improved inline documentation and comments throughout the code, clarifying the purpose of parameters like unsplit_name and providing default values in the example environment file. [1] [2]

@SFJohnson24 SFJohnson24 marked this pull request as ready for review December 19, 2025 22:04
@SFJohnson24 SFJohnson24 self-assigned this Dec 19, 2025
Copy link
Collaborator

@RamilCDISC RamilCDISC left a comment

Choose a reason for hiding this comment

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

The PR updates the handling for AP datasets. The validation was done by:

  1. Reviewing the PR for any unwanted code or comments.
  2. Reviewing the PR logic in accordance with AC.
  3. Ensuring all unit and regression testing pass.
  4. Ensure all relevant testing is updated.
  5. Ensuring the new tests cover edge cases.
  6. Running manual testing using dev rule editor on negative datasets.
  7. Running manual testing on dev rule editor using positive datasets.
  8. Matching the allowed variables with library.

@RamilCDISC RamilCDISC merged commit 8a439c5 into main Jan 2, 2026
17 of 18 checks passed
@RamilCDISC RamilCDISC deleted the AP_library branch January 2, 2026 21:38
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.

3 participants