Skip to content

feat: plain strings emit DeprecationWarning - #14

Merged
andremsouza merged 1 commit into
masterfrom
chore/deprecate-strings
Jul 30, 2026
Merged

feat: plain strings emit DeprecationWarning#14
andremsouza merged 1 commit into
masterfrom
chore/deprecate-strings

Conversation

@andremsouza

Copy link
Copy Markdown
Owner

The warning half of the string deprecation. 0.4.0 gave the written notice; this is what follows it,
and it is the last thing 1.0.0 needs before it can remove strings.

DeprecationWarning: Passing mode='batch' as a plain string is deprecated and will
stop working in 1.0.0. Use TrainingMode.BATCH instead.

The message names the exact replacement. A deprecation that makes the reader work out the
substitution is a deprecation people silence.

Two cases that deliberately stay silent

Both were found by building it, not by planning it.

An invalid string does not warn. My first version warned before validating, so under
filterwarnings = ["error"] a typo produced a DeprecationWarning instead of its ValueError
burying the real mistake under a notice telling the caller to modernise a spelling that never worked.
Reordering fixed the obvious cases but not sample_mode="cauchy", which is validated further in. The
rule that actually generalises is warn only for values the enum recognises, which removes the
ordering question entirely.

Loading a map does not warn. This one was a genuine design bug. load_npz reconstructs a map by
passing the stored strategy name to the constructor — and that name is a string, because JSON has
no enums. So opening any saved file warned the user about a string the file chose and they never
wrote
. The loader now converts it to the member it denotes. mexicanhat has no member of its own
and is an alias for the same function, so it maps to MEXICAN_HAT.

A regression this surfaced

Error messages interpolated the value directly, so passing an enum produced:

<WeightInit.LINEAR: 'linear'> initialization requires a 'data' argument

Both spellings now produce identical text, asserted by a test that compares the two.

The suite

filterwarnings = ["error"] means every string call site had to be dealt with rather than ignored.
55 sites moved to enum members across 8 files. The tests that exist to prove both spellings work
now assert the warning too, since that is the behaviour under test:

error_enum = first.train(blobs, n_iteration=15, mode=as_enum)
with pytest.warns(DeprecationWarning, match=f"mode={as_string!r}"):
    error_string = second.train(blobs, n_iteration=15, mode=as_string)
assert error_enum == error_string

Dedicated tests cover the deprecation itself: the message names the replacement for every spelling
including the legacy one, an invalid spelling raises instead of warning, an enum never warns, the
warning blames the caller's line rather than a line inside the package, and a stored artifact name
converts rather than warning.

Verification

384 tests, 100% coverage, ruff / ruff format / mypy --strict clean, mkdocs build --strict
exits 0, bandit clean, architecture profile --strict passes.

Not in this PR

The version bump. This is the change; the release is its own PR, as with 0.4.0.

0.4.0 deprecated the plain-string options in writing only, because `mode="batch"` was what every
documentation page showed at the time. This is the warning that follows the written notice, and the
last step before 1.0.0 removes strings entirely: the policy is that a removal in a major release
follows at least one minor release that warns.

The message names the exact replacement rather than describing one. A deprecation that makes the
reader work out the substitution is a deprecation people silence.

Two cases deliberately stay silent.

A string that names nothing does not warn. `mode="stochastic"` still raises its ValueError, rather
than burying the real mistake under a notice about modernising a spelling that never worked. The
first implementation warned before validating, which under `-W error` replaced the ValueError
outright; warning only for values the enum recognises removes the ordering question entirely.

Loading a map from a `.npz` does not warn. The name in an artifact is a serialisation detail, since
JSON has no enums, so handing it straight to the constructor would warn the caller about a string the
file chose and they never wrote. The loader converts it to the member it denotes. `mexicanhat` has no
member of its own and is an alias for the same function, so it maps to MEXICAN_HAT.

Also fixes error messages that showed an enum's repr when one was passed:
`weight_initialization(mode=WeightInit.LINEAR)` reported `<WeightInit.LINEAR: 'linear'>
initialization requires ...`. Both spellings now produce identical text.

The suite runs with `filterwarnings = ["error"]`, so 55 call sites move to enum members and the tests
that exercise both spellings now assert the warning as well as the equivalence. Dedicated tests cover
the deprecation itself: that the message names the replacement, that an invalid spelling raises
instead, that an enum never warns, and that the warning blames the caller's line.
@andremsouza
andremsouza merged commit da96f8b into master Jul 30, 2026
8 checks passed
@andremsouza
andremsouza deleted the chore/deprecate-strings branch July 31, 2026 12:31
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