Skip to content

update#1512

Merged
RamilCDISC merged 8 commits intomainfrom
codelist_att
Jan 12, 2026
Merged

update#1512
RamilCDISC merged 8 commits intomainfrom
codelist_att

Conversation

@SFJohnson24
Copy link
Copy Markdown
Collaborator

@SFJohnson24 SFJohnson24 commented Jan 5, 2026

This PR udpates codelist_attributes and its documents. I also briefly added the non_empty operator to the rule. I noticed the empty operator was not correctly identifying empty sets/lists/dicts because .isin() uses object identity/hash-based comparison for mutable collection types, causing it to fail to match empty collection instances (e.g., {}) despite the empty set literals in NULL_FLAVORS. I have added a length check for collections to also check if they are empty.

This pull request introduces several changes related to controlled terminology attribute extraction, schema validation, and test resources. The main focus is on updating the get_codelist_attributes operation to improve clarity and correctness, including changes to required parameters and documentation. Additionally, there are improvements to collection emptiness checks and minor formatting updates in test files.

Controlled Terminology Extraction and Schema Updates:

  • Updated the get_codelist_attributes operation in Operations.json to remove the target property from the required parameters and from the schema definition, reflecting a shift to using the name property for CT reference columns. [1] [2]
  • Revised the documentation in Operations.md to clarify how CT package names are constructed and to specify that the name parameter (not target) identifies the terminology system reference column. Also updated the example YAML and removed references to target. [1] [2]

DataFrame Operator Improvements:

  • Enhanced the empty method in dataframe_operators.py to check for empty collections (sets, lists, dicts) in addition to null flavors and NaNs, ensuring more robust detection of empty values.

Test Resource and Formatting Updates:

  • Cleaned up formatting in rule-286.json for better readability and consistency.
  • Updated the test rule in rule-286.json to remove the obsolete attribute property and align with the new schema by using only the version property.

Copy link
Copy Markdown
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.

I ran a validation using editor. I updated the rule to remove target and also added the non_empty check as you described in connected ticket. After these changes the negative dataset was reported as positive. I have attached the zip with all the artifacts to reproduce.

1512.zip

@SFJohnson24
Copy link
Copy Markdown
Collaborator Author

You need to give the CT packages in the library tab of the data if you are going to use editor @RamilCDISC

Copy link
Copy Markdown
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 codelist attribute to remove the target requirement. It also updates empty operator for stronger null/empty handlings. The PR was validated by:

  1. Reviewing the PR for any unwanted code or comments.
  2. Reviewing the PR logic in accordance iwth AC.
  3. Ensuring all unit and regression testing pass.
  4. Ensuring all related testing is updated.
  5. Ensuring updated testing covers edge cases.
  6. Running validation using CLI with positive dataset.
  7. Running validation using CLI with negative dataset.
  8. Running validation using rule editor with positive dataset.
  9. Running validation using rule editor with negative dataset.
  10. Ensuring cases of null and missing ct-packages.

@RamilCDISC RamilCDISC merged commit 72fb354 into main Jan 12, 2026
17 of 18 checks passed
@RamilCDISC RamilCDISC deleted the codelist_att branch January 12, 2026 22:42
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