Skip to content

Stata replication scripts may invert equal_to/not_equal_to row-removal conditions #246

Description

@NKeleher

Summary

The Stata do-file translation for the "remove row(s)" prep action appears to invert the row-removal direction for the value is equal to /
value is not equal to conditions, compared to what the real in-app Polars implementation actually does. The other four comparison conditions (greater than, greater than or equal to, less than, less than or equal to) are consistent between the two; only equality is not.

This means a generated 4_corrections.do / 3_prepare_data.do replication script that removes rows "where value is equal to X" (or "not equal to X") may keep/drop the opposite set of rows from what actually happened to the data inside DataSure.

Where

  • Real behavior (source of truth — this is what actually ran on the data):
    src/datasure/processing/prep.py,
    RemoveRowsOperation._filter_by_equality (~lines 297-315):

    if condition == PrepRowConditions.not_equal_to.value:
        # Keep rows where value is NOT in the list (remove matching rows)
        filter_expr = pl.any_horizontal([pl.col(col).is_in(value_list) for col in columns])
        return data.filter(~filter_expr)
    else:
        # Keep rows where value IS in the list (remove non-matching rows)
        filter_expr = pl.any_horizontal([pl.col(col).is_in(value_list) for col in columns])
        return data.filter(filter_expr)

    For condition == "value is equal to": keeps rows where the column
    equals the value (removes non-matching rows).

  • Stata translation:
    src/datasure/replication/prep_script_generator.py, _KEEP_OP (~lines
    48-54) and its use in _drop_by_condition (~line 136):

    _KEEP_OP: dict[str, str] = {
        _COND_GT: "<=",
        _COND_GTE: "<",
        _COND_LT: ">=",
        _COND_LTE: ">",
        _COND_EQ: "!=",
        _COND_NEQ: "==",
    }
    ...
    op = _KEEP_OP.get(condition)
    if op and values:
        fv = _fmt_val(values[0] if isinstance(values, list) else values)
        return [f"keep if {col} {op} {fv}"]

    For condition == "value is equal to", this generates keep if col != value, i.e. it removes rows where the column equals the value (keeps non-matching rows). That's the opposite of what prep.py actually did.

For the four comparison conditions (gt/gte/lt/lte), the _KEEP_OP flip direction matches RemoveRowsOperation._filter_by_comparisonexactly (e.g. "greater than" removescol > valuein both places). Onlyequal_to/not_equal_to` diverge.

Impact

Any replication package (2_scripts/3_prepare_data.do and/or 4_corrections.do, generated via prep_script_generator.py / script_generators.py) whose prep log contains a "remove row(s)" step with condition value is equal to or value is not equal to will likely reproduce the wrong set of rows when the Stata do-files are run, even though the same log replayed through the real Python pipeline (or the equivalent .py replication scripts added in #) reproduces the correct rows.

Suggested fix

Align _KEEP_OP (or wherever it's used) for _COND_EQ/_COND_NEQ with the real semantics in RemoveRowsOperation._filter_by_equality, OR confirm the intended semantics with whoever designed the "remove row(s), by condition" UI copy and fix whichever side is wrong. Add a regression test asserting the Stata-do-file translation and the real Polars removal agree on a small equal_to/not_equal_to example (e.g. via tests/replication/test_prep_script_generator.py).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    Fields

    No fields configured for Bug.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions