Repository navigation
Narwhalify conditions - #963
Open
AdrianSosic wants to merge 86 commits into
Open
AdrianSosic wants to merge 86 commits into
AdrianSosic wants to merge 86 commits into
Conversation
Replace the evaluate/to_polars interface with a more suitable __call__/_to_expr version: - __call__ is the user-facing entry point to evaluate the condition on an arbitrary input series - _to_expr offers the internal pathway for constructing lazy queries
Keep evaluate as the method returning the Boolean mask and preserve the input series name in the result.
Replace the index-based get_valid/get_invalid interface with a narwhals-based __call__ method that filters a native frame, preserving its backend and eager/lazy nature. Subclasses implement _to_expr for row-wise logic or _to_mask for frame-wise logic.
…rianceConstraint interfaces Implementation is temporarily dropped
Let filtering constraints apply exclude inversion in their base hooks while subclasses provide positive matching criteria. This keeps exclude handling centralized, so individual constraints cannot forget or duplicate the inversion while callers consistently consume final keep criteria.
Add a private FrameT-preserving filter path for internal Narwhals callers while retaining the native-frame public call adapter. This avoids a native roundtrip for internal lazy-frame filtering and preserves the eager/lazy frame kind.
This is a temporary workaround until continuous subset generation is refactored.
AdrianSosic
added
enhancement
Evaluate conditions through a shared Narwhals-based implementation, make conditions immutable, and split the condition tests into per-condition modules. Tighten dtype and missing value handling: Conditions now treat null and NaN values as non-matching. SubSelectionCondition rejects mixed-type and NaN selections, keeps Boolean entries instead of coercing them to floats, and raises on incompatible input dtypes. ThresholdCondition rejects Boolean data.
The constraint filtering pipeline is not narwhalified yet and still relies on the previous condition interface. Add temporary aliases for the renamed operator helpers, restore the Polars adapter (including consistent null/NaN handling and the tolerance fix for the "!=" operator), and cast the returned masks back to pandas series at the legacy call sites. The additive leftovers that would otherwise silently survive the migration are tagged with `TODO[bridge]`; the casts sit in pandas code paths that the new pipeline replaces wholesale.
AdrianSosic
force-pushed
the
refactor/narwhalify_conditions
branch
from
November 13, 2026 12:47
5973b4c to
82e9b6f
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Tolerance-window comparisons can misclassify values through rounding or overflow, and the condition strategy rejects valid mixed-type categorical domains.
2 open issues
What changed in this PR
This PR extracts the condition refactor from #953, moving BayBE’s conditions to a shared Narwhals-based API while retaining adapters for the existing constraint-filtering pipeline.
Changes:
- Unifies condition evaluation and filtering, and makes condition objects immutable.
- Defines dtype compatibility and missing-value rules, including Boolean subselections.
- Organizes tests by condition type and adds serialization roundtrips.
| File | Description |
|---|---|
tests/serialization/test_condition_serialization.py |
Adds condition serialization roundtrip tests. |
tests/hypothesis_strategies/conditions.py |
Expands generated selections and updates operator references. |
tests/constraints/utils.py |
Adds a shared full-test-environment marker. |
tests/constraints/conditions/utils.py |
Adds a backend-independent condition test helper. |
tests/constraints/conditions/test_threshold.py |
Tests threshold validation, operators, tolerances, and dtypes. |
tests/constraints/conditions/test_subselection.py |
Tests selection validation, filtering, and dtype compatibility. |
tests/constraints/conditions/test_base.py |
Tests shared null and NaN behavior. |
tests/constraints/conditions/conftest.py |
Provides pandas and Polars backend fixtures. |
tests/constraints/conditions/__init__.py |
Introduces the condition test package. |
CHANGELOG.md |
Documents immutability and condition behavior changes. |
baybe/utils/basic.py |
Adds a reusable instance-type validation helper. |
baybe/symmetries/dependency.py |
Adds a pandas type cast for condition masks. |
baybe/constraints/discrete.py |
Adds pandas type casts at legacy condition call sites. |
baybe/constraints/conditions.py |
Implements immutable Narwhals-based conditions with temporary Polars adapters. |
Review effort: Balanced
Exchanging the two interval bounds for a single absolute-difference comparison is simpler and avoids two numerical pitfalls of computing `threshold ± tolerance`: - At large magnitudes the bounds round outward, so a value can match despite being farther away than the tolerance. For example, with `threshold=10000000000000002.0` and `tolerance=1.0`, the value `10000000000000004.0` is 2.0 away but still matches. - When `threshold + tolerance` overflows to infinity, infinite values match a finite tolerance window, reversing the expected `!=` result.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
This PR is an extraction of #953, split off so that the work can be reviewed and merged in separate steps instead of one oversized PR. The bigger picture – motivation, design decisions, open questions, and the remaining steps – is described there; below is only a summary of the part moved into this PR.
Summary
__call__/evaluate/_to_exprpathConditionclasses immutablenullandNaNnever match a condition;SubSelectionConditionrejects mixed-type andNaNselections, keeps Boolean entries as Booleans (previously coerced to floats, which broke Polars filtering), and raises a descriptive error on incompatible input dtypes;ThresholdConditionrejects Boolean data instead of treating it as numericalBridging
Because the constraint filtering pipeline is only narwhalified in #953, this PR keeps the current pandas/Polars code paths working through temporary
TODO[bridge]-tagged adapters (legacy operator aliases, aCondition.to_polarsadapter, and pandas casts at the old call sites). They are removed again by #953.