Translate user-facing thinking_metric -> wire thinking_effort_metric#272
Merged
Conversation
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
The merged PRs renamed in different directions: - tabpfn-server (#852): server's FitRequest field is `thinking_effort_metric` - tabpfn-client (#270): client's user-facing kwarg is `thinking_metric` Result: any fit from `tabpfn-client@main` against `api.priorlabs.ai` 422s with `extra_forbidden` on `thinking_metric` — including plain v3/v2.5 fits, because the client always serialises the field as `null` even when thinking is off. Keeps the public API as `thinking_metric` (TabPFNClassifier/Regressor kwarg, `validate_thinking_mode`, the instance attribute), and only renames the FitRequest wire field + the call site that builds the request to `thinking_effort_metric` so the body matches the server's schema. Existing user code does not need to change.
c0f98f0 to
7dd0894
Compare
Contributor
There was a problem hiding this comment.
Code Review
This pull request renames the thinking_metric parameter to thinking_effort_metric throughout the library, affecting the API models, client implementation, classifier and regressor estimators, and associated unit tests. This update provides a more descriptive name for the optimization metric used during the fit process when thinking effort is specified. I have no feedback to provide.
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.
Summary
The merged renames went different directions on the two repos:
tabpfn-server(#852, deployed as prod gapi) — wire field isthinking_effort_metrictabpfn-client(feat: thinking-mode knobs (rename from enhanced_*) #270, this main) — public kwarg + wire field isthinking_metricResult: any fit from
tabpfn-client@mainagainstapi.priorlabs.ai422s withextra_forbiddenonthinking_metric— including plain v3 and v2.5 fits, because the field is serialised asnulleven when thinking is off.This PR keeps the public API as
thinking_metric(theTabPFNClassifier/TabPFNRegressorkwarg, the instance attribute, andvalidate_thinking_mode) so existing user code does not change. Only the wire field onFitRequestand the call site that builds the request body are renamed tothinking_effort_metricto match the server's schema.Diff is +4/-2 across 2 files.
Test plan
pytest tests/unit/— 124/124 passingapi.priorlabs.aifor v3 plain, v3 thinking medium, v3 thinking high, v2.5 plain (both classification and regression)