Skip to content

Fix inverted non-numeric >= and <= fallbacks in resolve_filter - #393

Open
nikhilsi wants to merge 1 commit into
larsiusprime:masterfrom
nikhilsi:fix-string-ordered-filters
Open

Fix inverted non-numeric >= and <= fallbacks in resolve_filter#393
nikhilsi wants to merge 1 commit into
larsiusprime:masterfrom
nikhilsi:fix-string-ordered-filters

Conversation

@nikhilsi

@nikhilsi nikhilsi commented Jul 4, 2026

Copy link
Copy Markdown

What was wrong

In openavmkit/filters.py at 7952236, the non-numeric fallback branches for
>= and <= are swapped:

  • line 193: >= on a non-numeric column returns df[field].le(value)
  • line 198: <= on a non-numeric column returns df[field].ge(value)

The > and < branches just above (lines 183, 188) are correct, which
points to a copy-paste swap. The effect is that any settings filter that
compares a string column with >= or <= (the common case being sale-date
thresholds like [">=", "sale_date", "str:2020-01-01"]) silently selects
the exact complement of the intended rows.

The change

Swap them back: >= maps to .ge(value) and <= maps to .le(value).
Two lines. Numeric columns are unaffected (they take the
is_column_of_type branch, which was already correct).

What changes downstream

Filters on non-numeric columns using >= or <= start selecting the rows
they say they select. Any settings file that unknowingly relied on the
inverted behavior will flip, but that behavior contradicted both the
operator names and the numeric branch, so the previous results were wrong.

Verification

A test is included (tests/test_filters.py). It builds a frame with a
string sale_date column spanning three dates and asserts that
[">=", "sale_date", "str:<middle date>"] selects the middle and later
rows and ["<=", ...] selects the middle and earlier rows. Both assertions
fail at 7952236 and pass with this change.

Fixes #376.

The non-numeric branch of >= returned le() and <= returned ge(), so
any string or date ordered comparison silently selected the opposite
rows. Adds truth-table tests for string dates including > and < as
already-correct controls.
@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution.
Please sign our CLA at the following link:
Click here to sign the CLA.

No action is required from you in this PR thread. Once you have signed the CLA externally, a maintainer will verify your signature and record it here on your behalf by commenting:


I affirm that this contributor has signed the CLA


You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

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.

Non-numeric >= and <= filters return the opposite rows

1 participant