Skip to content

feat: make max length argument explicit - #389

Merged
stephantul merged 2 commits into
mainfrom
explicit-max-length
Oct 5, 2026
Merged

stephantul merged 2 commits into
mainfrom
explicit-max-length

Conversation

@stephantul

Copy link
Copy Markdown
Contributor

The max length argument between StaticModel and the trainable counterparts was inconsistent: for StaticModel, we use an unset sentinel to mean "do what the model does" and None to mean no max_length. For trainable models, it was impossible to unset the max length.

@greptile-apps

greptile-apps Bot commented Oct 5, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[Medium risk] Changes how max length parameter defaults are handled across training classes.

The PR appears safe to merge; an end-to-end truncation test would provide useful non-blocking coverage.

Reviews (1) · Last reviewed commit: "feat: make max length argument explicit"

Comment thread tests/test_trainable.py Outdated
@codecov

codecov Bot commented Oct 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

Files with missing lines Coverage Δ
model2vec/train/base.py 99.56% <100.00%> (+<0.01%) ⬆️
model2vec/train/classifier.py 100.00% <100.00%> (ø)
model2vec/train/pairs.py 100.00% <100.00%> (ø)
model2vec/train/similarity.py 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stephantul
stephantul merged commit 582d54a into main Oct 5, 2026
11 checks passed
@stephantul
stephantul deleted the explicit-max-length branch October 5, 2026 19:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant