Skip to content

AutoBool allow_* flags#742

Open
AdrianSosic wants to merge 10 commits intomainfrom
refactor/allow_flags
Open

AutoBool allow_* flags#742
AdrianSosic wants to merge 10 commits intomainfrom
refactor/allow_flags

Conversation

@AdrianSosic
Copy link
Copy Markdown
Collaborator

@AdrianSosic AdrianSosic commented Feb 10, 2026

Fixes #733.

The drawback of the previous approach was that the flag attributes had a union bool | UnspecifiedType type, which meant that they could not be safely accessed without additional conversion logic, resulting in the bug reported in #733.

To fix this, the user-facing value of the flags should always be a simple bool at query time, while still offering a clever "auto-determine" mechanism that does not require the user to explicitly set them depending on the search space type.

There are two options to achieve this:

  1. Using a context-specific conversion at attribute setting time. While possible, it has the disadvantage that it complicates validation, which usually happens only after conversion.
  2. For this reason, we use an AutoBool approach, where the user-specified value is stored as a private attribute and the public value is resolved lazily at query time.

@AdrianSosic AdrianSosic self-assigned this Feb 10, 2026
Copilot AI review requested due to automatic review settings February 10, 2026 08:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Updates Campaign.allow_* flags to use an AutoBool mechanism so callers always observe plain bool values at query time, while still supporting an “auto” mode.

Changes:

  • Replaced bool | UnspecifiedType allow-flag storage with private AutoBool attrs fields plus public bool-returning properties.
  • Updated validation to raise IncompatibilityError for incompatible allow-flag settings when continuous subspaces are involved.
  • Refreshed tests and changelog to cover/describe the new behavior (including "auto").

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File Description
tests/test_campaign.py Updates parametrization and assertions to validate lazy resolution of allow flags and new error type.
baybe/campaign.py Introduces AutoBool-backed private fields, new validator, and public bool properties for allow flags.
CHANGELOG.md Documents the behavioral change and a cache-related fix.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@AVHopp
Copy link
Copy Markdown
Collaborator

AVHopp commented Feb 10, 2026

@AdrianSosic: The create_from_config example seems to be failing, causing the docs fail

Copy link
Copy Markdown
Collaborator

@AVHopp AVHopp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except for the failing tests this looks good to me.

@AdrianSosic AdrianSosic force-pushed the refactor/allow_flags branch from e02883f to d721d8b Compare March 30, 2026 11:42
@AdrianSosic AdrianSosic force-pushed the refactor/allow_flags branch from 551cf1e to cde2954 Compare March 31, 2026 07:49
@AdrianSosic AdrianSosic force-pushed the refactor/allow_flags branch from cde2954 to c31e6ec Compare March 31, 2026 08:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bugged recommendation cache check accessing allow_recommending_already_recommended when it's UNSPECIFIED

4 participants