fixes by SFJohnson24 · Pull Request #1619 · cdisc-org/cdisc-rules-engine

SFJohnson24 · 2026-02-16T21:37:27Z

This PR

removes the unused rule type from docs, tests and code as it does not fit the factory archetype that has been implemented, has no rules for it.
removes the VLM operators that were also unused
correctly maps Record Data rule type to Contents Builder //changed the get_service call to raise an error if the rule_type does not correctly match a rule, instead of defaulting to a potential incorrect rule type
added record data to tests that had no rule type and were falling into this default pattern
restores Rule Macro for each rule type
removes the old code in rule_engine for rule types.
updates the update-cache docs
removes add_comparator_to_rule_conditions was injecting comparators into rules/silently mutating them? From the test it seems it would grab literal values from metadata values

RamilCDISC · 2026-02-17T20:15:26Z

cdisc_rules_engine/dataset_builders/dataset_builder_factory.py

        RuleTypes.DOMAIN_PRESENCE_CHECK_AGAINST_DEFINE.value: DomainListWithDefineDatasetBuilder,
        RuleTypes.DEFINE_ITEM_METADATA_CHECK.value: DefineVariablesDatasetBuilder,
        RuleTypes.VARIABLE_METADATA_CHECK_AGAINST_DEFINE.value: VariablesMetadataWithDefineDatasetBuilder,
-        RuleTypes.DATASET_CONTENTS_CHECK_AGAINST_DEFINE_AND_LIBRARY.value: ContentsDatasetBuilder,


Could you please update the documentation of Rule Type to remove this from there too?

Are we planning to keep it in the documentation and remove form code? @SFJohnson24

odd I had though I removed it. It is done @RamilCDISC

RamilCDISC · 2026-02-17T20:19:07Z

README.md


 To obtain an api key, please follow the instructions found here: <https://wiki.cdisc.org/display/LIBSUPRT/Getting+Started%3A+Access+to+CDISC+Library+API+using+API+Key+Authentication>. Please note it can take up to an hour after sign up to have an api key issued

+This will show the list of cache update options.


I believe this line is incomplete. The command it is referring to is missing from here

line is a bit strangely worded. changed it.

…les-engine into issue_reports

RamilCDISC · 2026-02-24T19:36:42Z

resources/schema/rule/Rule_Type.md

@@ -1,5 +1,3 @@
-## Dataset Contents Check against Define XML and Library Metadata
-
 #### Columns


Could you please remove the description of the rule type too? The title is removed but sub headings are still there.

I do not see the sub headings you are referring to @RamilCDISC

RamilCDISC

The PR does cleaning of the codebase by removing unused rule types and operators. It also updates the documentation. the validation was done by:

Reviewing the PR for any unwanted code or changes.
Validating all tests pass.
Ensuring removed rule type documentation and tests are properly removed.
Ensuring removed operators tests and documentation is removed too.
validating update cache documentation is updated.
Ensuring CORE test suite validation pass.
Verifying readability of the updated documentation.

gerrycampion

It looks like this resolves part of ticket #514 as well. Maybe take a look to see if it makes sense to address anything else from that ticket. I don't think we want to move forward with removing the items in the Operations section

cdisc_rules_engine/rules_engine.py

tests/unit/test_dataset_builders/test_contents_dataset_builder.py

cdisc_rules_engine/check_operators/dataframe_operators.py

.flake8

…les-engine into issue_reports

gerrycampion · 2026-03-04T21:55:58Z

cdisc_rules_engine/dataset_builders/dataset_builder_factory.py

        RuleTypes.VARIABLE_METADATA_CHECK_AGAINST_LIBRARY.value: VariablesMetadataWithLibraryMetadataDatasetBuilder,
        RuleTypes.DEFINE_ITEM_METADATA_CHECK_AGAINST_LIBRARY.value: DefineVariablesWithLibraryMetadataDatasetBuilder,
-        RuleTypes.VARIABLE_METADATA_CHECK_AGAINST_DEFINE_XML_AND_LIBRARY.value: VariablesMetadataWithDefineAndLibraryDatasetBuilder,
+        RuleTypes.VARIABLE_METADATA_CHECK_AGAINST_DEFINE_XML_AND_LIBRARY.value: VariablesMetadataWithDefineAndLibraryDatasetBuilder,  # noqa: E501


why does this need to be here now if it is in flake8?

removed this and several other inline # noqa: E501

fixes

0e2af26

SFJohnson24 temporarily deployed to DEV February 16, 2026 21:37 — with GitHub Actions Inactive

remove test

d38d932

SFJohnson24 requested review from RamilCDISC and gerrycampion February 16, 2026 21:42

SFJohnson24 linked an issue Feb 16, 2026 that may be closed by this pull request

--apikey option in addition to setting a local environment variable #1581

Closed

Merge branch 'main' into issue_reports

77014a7

RamilCDISC requested changes Feb 17, 2026

View reviewed changes

SFJohnson24 added 2 commits February 19, 2026 16:32

Merge branch 'main' into issue_reports

2fd4ce7

update line

e910858

SFJohnson24 requested a review from RamilCDISC February 19, 2026 21:35

SFJohnson24 and others added 5 commits February 19, 2026 16:35

Merge branch 'issue_reports' of https://github.com/cdisc-org/cdisc-ru…

b8b2143

…les-engine into issue_reports

Merge branch 'main' into issue_reports

8a45d80

Merge branch 'main' into issue_reports

70b30b5

docs

5302bb7

Merge branch 'issue_reports' of https://github.com/cdisc-org/cdisc-ru…

71404bc

…les-engine into issue_reports

RamilCDISC requested changes Feb 24, 2026

View reviewed changes

Merge branch 'main' into issue_reports

281c6f5

SFJohnson24 requested a review from RamilCDISC February 26, 2026 15:38

SFJohnson24 added 2 commits February 26, 2026 13:42

spring cleaning

f3c839f

spring cleaning

b3de2bd

SFJohnson24 self-assigned this Feb 26, 2026

SFJohnson24 added 3 commits February 26, 2026 14:43

restore record data

2302357

tests

bc6e5de

remove

b04f8c3

RamilCDISC approved these changes Feb 26, 2026

View reviewed changes

Merge branch 'main' into issue_reports

871b2be

gerrycampion requested changes Feb 27, 2026

View reviewed changes

SFJohnson24 linked an issue Mar 2, 2026 that may be closed by this pull request

Deprecate operators/operations/rule types that have preferred alternatives #514

Open

restored test, removed artifacts

2d77354

SFJohnson24 temporarily deployed to DEV March 2, 2026 19:22 — with GitHub Actions Inactive

update to factory

7339967

SFJohnson24 temporarily deployed to DEV March 2, 2026 19:44 — with GitHub Actions Inactive

get linters to read .flake8

de3ea9f

SFJohnson24 temporarily deployed to DEV March 2, 2026 19:48 — with GitHub Actions Inactive

ignore

b090426

SFJohnson24 temporarily deployed to DEV March 2, 2026 19:53 — with GitHub Actions Inactive

restore line enforcement

5bab3c7

SFJohnson24 requested a review from gerrycampion March 2, 2026 19:58

Merge branch 'main' into issue_reports

a48c69e

SFJohnson24 temporarily deployed to DEV March 2, 2026 21:28 — with GitHub Actions Inactive

gerrycampion requested changes Mar 4, 2026

View reviewed changes

.flake8 Outdated Show resolved Hide resolved

.flake8 Outdated Show resolved Hide resolved

.flake8 Outdated Show resolved Hide resolved

SFJohnson24 added 2 commits March 4, 2026 13:34

flake8 changes

7e82db0

Merge branch 'issue_reports' of https://github.com/cdisc-org/cdisc-ru…

11004a0

…les-engine into issue_reports

SFJohnson24 temporarily deployed to DEV March 4, 2026 18:35 — with GitHub Actions Inactive

SFJohnson24 requested a review from gerrycampion March 4, 2026 18:37

gerrycampion requested changes Mar 4, 2026

View reviewed changes

SFJohnson24 added 2 commits March 5, 2026 06:17

merge main

22fd726

removed inline e501 exceptions

7bac61c

SFJohnson24 temporarily deployed to DEV March 5, 2026 11:19 — with GitHub Actions Inactive

SFJohnson24 requested a review from gerrycampion March 5, 2026 11:19

Merge branch 'main' into issue_reports

1254757

SFJohnson24 temporarily deployed to DEV March 5, 2026 21:26 — with GitHub Actions Inactive

Merge branch 'main' into issue_reports

8077ec4

SFJohnson24 temporarily deployed to DEV March 11, 2026 13:55 — with GitHub Actions Inactive

prettier operator.md

1d60d17

gerrycampion temporarily deployed to DEV March 11, 2026 18:58 — with GitHub Actions Inactive

gerrycampion approved these changes Mar 11, 2026

View reviewed changes

gerrycampion merged commit c02cbdb into main Mar 11, 2026
12 of 13 checks passed

gerrycampion deleted the issue_reports branch March 11, 2026 19:20


		To obtain an api key, please follow the instructions found here: <https://wiki.cdisc.org/display/LIBSUPRT/Getting+Started%3A+Access+to+CDISC+Library+API+using+API+Key+Authentication>. Please note it can take up to an hour after sign up to have an api key issued

		This will show the list of cache update options.

		@@ -1,5 +1,3 @@
		## Dataset Contents Check against Define XML and Library Metadata

		#### Columns

Conversation

SFJohnson24 commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RamilCDISC left a comment

Choose a reason for hiding this comment

Uh oh!

gerrycampion left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

SFJohnson24 commented Feb 16, 2026 •

edited

Loading