Constraint Refactor 3: Merge *NoLabelDuplicates* and *LinkedParameters* into *Repetition* - #881
Constraint Refactor 3: Merge *NoLabelDuplicates* and *LinkedParameters* into *Repetition*#881Scienfitz wants to merge 28 commits into
*NoLabelDuplicates* and *LinkedParameters* into *Repetition*#881Conversation
5321409 to
ce150d1
Compare
There was a problem hiding this comment.
Pull request overview
This PR completes the constraint refactor by unifying discrete “pruning” semantics (specification describes what is kept, invertible via exclude=True) and consolidating the former DiscreteNoLabelDuplicatesConstraint / DiscreteLinkedParametersConstraint into a single DiscreteDegeneracyConstraint, while preserving backward compatibility via deprecations and serialization redirects.
Changes:
- Introduce
DiscretePruningConstraintwith a uniformexcludeflag and matching-rows (“kept rows”) implementations across discrete pruning constraints. - Replace
DiscreteExcludeConstraintwithDiscreteFilteringConstraint(and deprecate the old name), and merge label-duplicate / linked-parameter constraints intoDiscreteDegeneracyConstraint(with deprecation + cattrs redirects). - Update search space construction, tests, docs, examples, and the pruning order constant rename (
DISCRETE_CONSTRAINTS_*_ORDER).
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_deprecations.py |
Adds deprecation + legacy-deserialization coverage for renamed/merged constraints. |
tests/test_campaign.py |
Updates campaign tests to use DiscreteFilteringConstraint(..., exclude=True) instead of the deprecated exclude constraint. |
tests/serialization/test_constraint_serialization.py |
Updates serialization property tests/strategies to reflect new constraint classes. |
tests/hypothesis_strategies/constraints.py |
Reworks Hypothesis generators to emit DiscreteFilteringConstraint and DiscreteDegeneracyConstraint. |
tests/constraints/test_constraints_polars.py |
Switches Polars constraint tests to the new degeneracy constraint (including “linked” behavior via exclude=True). |
tests/constraints/test_constrained_cartesian_product.py |
Renames ordering constant usage and updates scenarios to new constraints. |
tests/constraints/test_batch_constraint.py |
Updates batch-constraint tests to use filtering constraints with exclude=True. |
tests/conftest.py |
Updates shared fixtures to use new constraint classes and semantics. |
examples/Mixtures/slot_based.py |
Updates example usage from no-label-duplicates to degeneracy constraint. |
examples/Constraints_Discrete/filtering_constraints.py |
Renames/updates the example to show filtering constraints and the new exclude behavior. |
docs/concepts/getting_recommendations.md |
Updates documentation example to use DiscreteFilteringConstraint(..., exclude=True). |
docs/components/constraints.md |
Refactors docs to explain pruning constraints + exclude, and documents DiscreteFilteringConstraint and DiscreteDegeneracyConstraint. |
CHANGELOG.md |
Documents added/changed/deprecated/removed items for the constraint refactor. |
baybe/searchspace/utils.py |
Applies pruning constraints by type, updates ordering, and adapts Polars/pandas filtering to the new interface. |
baybe/searchspace/discrete.py |
Updates constraint ordering logic to the new pruning order constant. |
baybe/constraints/discrete.py |
Implements new/renamed discrete pruning constraints, adds DiscreteDegeneracyConstraint, and adds deprecation factories + legacy structuring redirects. |
baybe/constraints/base.py |
Introduces DiscretePruningConstraint ABC with exclude semantics and matching-row APIs (pandas + optional Polars). |
baybe/constraints/__init__.py |
Exposes new constants/classes and maintains deprecated exports. |
baybe/campaign.py |
Updates candidate toggling to accept pruning constraints (import + runtime type narrowing). |
Suppressed comments (1)
baybe/constraints/discrete.py:72
DiscreteFilteringConstraintdoes not validate thatconditionsandparametershave the same length. This can silently ignore parameters (viazip(...)) or raiseIndexErrorin the Polars path (self.parameters[k]). Add an attrs validator to enforce a 1:1 mapping at construction time.
conditions: list[Condition] = field(validator=min_len(1))
"""List of individual conditions."""
combiner: str = field(default="AND", validator=in_(_valid_logic_combiners))
"""Operator encoding how to combine the individual conditions."""
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
4fda3cb to
6cab715
Compare
AVHopp
left a comment
There was a problem hiding this comment.
Minor comment, but looks quite good to me. Full review once previous PR is merged.
6cab715 to
4d56c5d
Compare
9784031 to
ca1a744
Compare
AdrianSosic
left a comment
There was a problem hiding this comment.
More things coming, but here already something to work on
AdrianSosic
left a comment
There was a problem hiding this comment.
remaining code comments, now looking at doc
f6be967 to
a0c3b0f
Compare
*NoLabelDuplicates* and *LinkedParameters* into *Degeneracy**NoLabelDuplicates* and *LinkedParameters* into *Repetition*
Passing 'exclude' to the deprecated alias raised a TypeError because it collided with the fixed 'exclude=True' forwarded to the replacement.
The Polars implementation silently diverged from the pandas result for heterogeneous value types (e.g. a categorical '1' vs. a numeric 1.0), which are distinct under pandas but were collapsed by Polars. Encode values with a type tag so both backends produce identical filtering.
Co-authored-by: AdrianSosic <adrian.sosic@merckgroup.com>
DiscreteRepetitionConstraint now has a Polars implementation, so the test no longer exercised the mixed routing path.
9314527 to
b4a40d2
Compare
| class DiscreteNoLabelDuplicatesConstraint(DiscreteFilteringConstraint): | ||
| """Constraint class for keeping entries where all labels are unique. | ||
| @define | ||
| class DiscreteRepetitionConstraint(DiscreteFilteringConstraint): |
There was a problem hiding this comment.
we now need to rethink the name, becasue the convention is that the name expresses what is kept, but with default configuration this cosntraints removes repetitions
Simplest suggestions for me:
DiscreteNoRepetitionConstraintDiscreteRepetitionLimitConstraint
There was a problem hiding this comment.
First variant makes more sense to me - immediately makes clear that no repetitions are kept while the other one would is a bit more obscure imo
There was a problem hiding this comment.
I slightly favor the second one, simply because it also remains valid for all values of n_max_repetitions (its always a limit on the repetitions)
Expose the Polars namespace to constraint modules during documentation builds so sphinx-autodoc-typehints can evaluate pl.Schema without adding an eager Polars import to the library.
b4a40d2 to
1e9600e
Compare
Closes #874
Based on #880