Skip to content

Narwhalify DiscreteParameter.transform - #862

Merged
AdrianSosic merged 36 commits into
dev/candidatesfrom
refactor/encoded_parameters
Jul 21, 2026
Merged

AdrianSosic merged 36 commits into
dev/candidatesfrom
refactor/encoded_parameters

Conversation

@AdrianSosic

@AdrianSosic AdrianSosic commented Jul 10, 2026 •

Copy link
Copy Markdown
Collaborator

Cleans up the discrete parameter hierarchy and broadens the transform interface toward narwhals-native evaluation, laying the groundwork for backend-agnostic computational representations.

Background

The long-term goal is to make DiscreteParameter.transform fully backend-agnostic: accept any narwhals-compatible input and return a corresponding native eager dataframe. This PR implements the interface change and migrates NumericalDiscreteParameter and CategoricalParameter fully; SubstanceParameter and CustomDiscreteParameter retain a pandas-backed _comp_df fallback pending a follow-up.

Several related cleanups are bundled here because they are prerequisites or natural companions: the encoding attribute was spuriously placed on the base class, CustomEncoding was a CategoricalEncoding enum value that never applied to CustomDiscreteParameter (whose "encoding" is the user-supplied DataFrame itself, not an enum choice), several parameter fields lacked validators/converters, and validate_parameter_input had an iterrows-based loop that caused significant slowdowns once TableCandidates began calling it at construction time.

- Drop the spurious ParameterEncoding | None placeholder field from
  DiscreteParameter; encoding is only meaningful on concrete label-like
  subclasses that actually use it
- Remove the vestigial CustomEncoding field from CustomDiscreteParameter;
  the encoding is the user-supplied data DataFrame itself, so the field
  carried no information and was never read
- Simplify DiscreteParameter.transform() to the identity path
  (series.to_frame()); move the comp_df merge logic into a new
  transform() override on _DiscreteLabelLikeParameter
@AdrianSosic AdrianSosic added this to the 0.16.0 milestone Jul 10, 2026
@AdrianSosic AdrianSosic self-assigned this Jul 10, 2026
@AdrianSosic AdrianSosic added enhancement Expand / change existing functionality refactor dev labels Jul 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.

Comment thread baybe/parameters/categorical.py Resolved
Comment thread baybe/parameters/categorical.py Resolved
Comment thread baybe/parameters/base.py Resolved
Comment thread CHANGELOG.md Resolved
Comment thread CHANGELOG.md Resolved
@AdrianSosic AdrianSosic mentioned this pull request Jul 10, 2026
11 tasks
Comment thread baybe/parameters/base.py Resolved
Comment thread baybe/parameters/categorical.py Resolved
@AdrianSosic AdrianSosic changed the title Narwhalify and lazify DiscreteParameter.transform Narwhalify DiscreteParameter.transform Jul 17, 2026
@AdrianSosic
AdrianSosic force-pushed the refactor/encoded_parameters branch from 916f2c2 to 40e2d0f Compare July 17, 2026 09:16
@AdrianSosic
AdrianSosic force-pushed the refactor/encoded_parameters branch from 40e2d0f to 0cdd025 Compare July 17, 2026 09:25
Zero coefficients are now rejected (incoming change from main)
Comment thread baybe/parameters/base.py Resolved
Comment thread baybe/parameters/base.py Resolved
Comment thread baybe/parameters/custom.py Resolved
Comment thread baybe/parameters/custom.py Resolved
Comment thread baybe/parameters/substance.py Resolved
Comment thread tests/test_parameters.py Resolved
Comment thread baybe/parameters/base.py Resolved
Comment thread baybe/parameters/custom.py Resolved
Comment thread baybe/parameters/custom.py Resolved
Co-authored-by: Myra <myra.zmarsly@outlook.com>
@AdrianSosic
AdrianSosic force-pushed the refactor/encoded_parameters branch from 5ebac99 to 07f9025 Compare July 20, 2026 10:43

@myrazma myrazma left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good no me now as all my comments & questions are resolved. As discussed, some parts need to be revisit later as described in #837.

@fabianliebig fabianliebig left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM. I also think it make sense to first create a first version and then adjust and see how narwahl can be implemented best :)

Comment thread tests/test_substance_parameter.py Resolved
@AdrianSosic
AdrianSosic merged commit 707f443 into dev/candidates Jul 21, 2026
11 of 13 checks passed
@AdrianSosic
AdrianSosic deleted the refactor/encoded_parameters branch July 21, 2026 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev enhancement Expand / change existing functionality refactor

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants