feat: add does_not_start_with and does_not_end_with operators#1791
feat: add does_not_start_with and does_not_end_with operators#1791Rdag15 wants to merge 3 commits into
Conversation
Adds the negated complements of the existing starts_with / ends_with string operators, following the established negation pattern in this module (e.g. does_not_contain -> ~self.contains(...)). - dataframe_operators.py: does_not_start_with / does_not_end_with - resources/schema/rule/Operator.json: operator enum entries - resources/schema/rule/Operator.md: operator documentation - resources/schema/rule-merged/Operator.json: regenerated via scripts/merge_schema_markdown.py + prettier (no manual edits) - tests/unit/test_check_operators/test_string_comparison.py: parametrized tests across PandasDataset and DaskDataset Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
| @type_operator(FIELD_DATAFRAME) | ||
| def does_not_end_with(self, other_value): | ||
| return ~self.ends_with(other_value) | ||
|
|
There was a problem hiding this comment.
Starts_with and ends_with currently returns raw result of pandas string methods. These methods can produce nullable/objects results when target values is missing. This can mix badly with use of ''. Other operators safely use '' when positive has already normalized its output to boolean series. Please make starts_with and ends_with null safe.
There was a problem hiding this comment.
Fixed in f865a42. Confirmed: with a missing target value, starts_with returned [True, None, False] (object dtype) and does_not_start_with then raised TypeError: bad operand type for unary ~: 'NoneType'. Both starts_with and ends_with now pass na=False to str.startswith/str.endswith, so the result is always a clean boolean Series (missing -> False) that negates safely. Verified on PandasDataset and DaskDataset.
There was a problem hiding this comment.
Could you please add cases for when 'value_is_literal=True'?
There was a problem hiding this comment.
Added in f865a42: test_starts_with_literal and test_ends_with_literal cover value_is_literal=True for both operators and their negations, plus test_does_not_start_with_is_null_safe / test_does_not_end_with_is_null_safe for the missing-value case — all parametrized across PandasDataset and DaskDataset.
Addresses review on cdisc-org#1791. - starts_with/ends_with returned raw pandas string-method output, which is object/NaN when target values are missing. Negating that (does_not_start_with = ~self.starts_with) raised "bad operand type for unary ~: 'NoneType'". Pass na=False so the result is always a clean boolean Series (missing -> False), matching how other negating operators rely on a normalized boolean input. - Add tests for value_is_literal=True (starts/ends and their negations) plus null-safety regression tests, across PandasDataset and DaskDataset. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
RamilCDISC
left a comment
There was a problem hiding this comment.
The PR adds does_not_start and does_not_end with operators to CORE. These operators are negatives of starts_with and ends_with operators. The PR was validated by:
- Reviewing the PR for any unwanted code or comments.
- Reviewing the PR in accordance to the AC.
- Reviewing the code to verify it follows the previous implemented code standards.
- Ensuring the documentation is updated.
- Ensuring the documentation mentions the use and explanation correctly.
- Ensuring the schema is updated.
- Ensuring previous tests are updated and new tests are added for the new operators.
- Ensuring the new tests cover null and empty cases.
- Ensuring the existing operators are updated to work with '~' in the negative operators.
- Ensuring all unit and regression tests pass.
| {"target": ["Att", "Btt", "Ctt"], "VAR2": ["tt", "B", "D"]}, | ||
| "VAR2", | ||
| PandasDataset, | ||
| [False, False, False], |
There was a problem hiding this comment.
Am I misunderstanding, or shouldn't these be [False, True, True]
Summary
Adds two new string operators,
does_not_start_withanddoes_not_end_with, the negated complements of the existingstarts_with/ends_withoperators.Closes #1153.
Implementation
The operators follow the negation pattern already established in
dataframe_operators.py(e.g.does_not_containis~self.contains(...)anddoes_not_equal_string_partis~self.equals_string_part(...)):Changes
cdisc_rules_engine/check_operators/dataframe_operators.py— the two operator methodsresources/schema/rule/Operator.json— operator enum entriesresources/schema/rule/Operator.md— operator documentationresources/schema/rule-merged/Operator.json— regenerated withpython scripts/merge_schema_markdown.pyfollowed bynpx prettier ... --write(not hand-edited), so it stays in sync withcheck-schema-markdowntests/unit/test_check_operators/test_string_comparison.py— parametrized tests for both operators acrossPandasDatasetandDaskDatasetTesting
pytest tests/unit/test_check_operators/test_string_comparison.py— 77 passed (incl. 4 new cases)black --check— cleanflake8— cleancheck-schema-markdownstays green