From 2e2cbe31b060e3357a0ea12e22f74252ef1596c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Moreira=20Souza?= Date: Thu, 30 Jul 2026 16:43:43 -0300 Subject: [PATCH] fix: withdraw the plain-string deprecation 0.5.0 made plain strings emit DeprecationWarning and announced removal in 1.0.0. That was wrong. Strings are now permanently supported, and 1.0.0 will not remove them. Every comparable library passes options as plain strings, and none export enums: scikit-learn (KMeans(init="k-means++", algorithm="lloyd")), numpy (np.pad(mode="constant")), scipy (linkage(method="single")), and both SOM libraries, minisom (neighborhood_function="gaussian") and sompy (normalization="var"). scikit-learn validates them at runtime through StrOptions, a constraint rather than a type. Removing the string form would have made this the only library in its ecosystem to reject mode="batch". The argument originally made for enums, from Viafore's Robust Python ch. 8, is about preventing typos inside a codebase, and he adds a "When Not to Use" section. The benefit he claims is that a type checker rejects a wrong value, and the Literal unions added in 0.4.0 already deliver exactly that with strings still working: mode="bacth" is a type error while mode="batch" is not. The deprecation bought nothing that was not already had. The enums stay. They cost nothing, some callers prefer them, and removing them would be a second reversal in two releases. `warn_if_string` and its four call sites are gone. The tests that wrapped string calls in `pytest.warns` are unwrapped, and a new test asserts the *absence* of any warning at every call site under `simplefilter("error")`, so the withdrawal cannot be quietly undone. The decision was originally made without checking what comparable libraries do. --- CHANGELOG.md | 21 ++++++ docs/reference/options.md | 50 +++++-------- src/python_som/_enums.py | 61 +++------------- src/python_som/_som.py | 6 -- tests/test_typed_seams.py | 149 +++++++++++++------------------------- 5 files changed, 101 insertions(+), 186 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0525dc5..7dbd14a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,27 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +- **The plain-string deprecation introduced in 0.5.0 is withdrawn.** Strings are permanently + supported, the `DeprecationWarning` is gone, and 1.0.0 will not remove them. If you migrated to + the enums while 0.5.0 was current, nothing you wrote breaks: the enums are staying too. + + 0.5.0 was wrong, and the reason is worth stating rather than quietly reverting. Every comparable + library passes options as plain strings and none export enums: scikit-learn + (`KMeans(init="k-means++")`), numpy (`np.pad(mode="constant")`), scipy + (`linkage(method="single")`), and both SOM libraries, minisom and sompy. Removing the string form + would have made this the only library in its ecosystem to reject `mode="batch"`. + + The benefit originally claimed for enums was that a type checker catches typos. That is already + delivered by the `Literal` unions added in 0.4.0, with strings still working: `mode="bacth"` is a + type error while `mode="batch"` is not. The deprecation bought nothing that was not already had. + + The decision was made without checking what comparable libraries do. That check is now part of + planning any future API change. + ## [0.5.0] - 2026-07-30 One change: the plain-string options now warn. Nothing else moves, and no result changes. diff --git a/docs/reference/options.md b/docs/reference/options.md index 79c50ec..619644c 100644 --- a/docs/reference/options.md +++ b/docs/reference/options.md @@ -60,40 +60,30 @@ when the value comes from outside the program. The legacy spelling `"mexicanhat"` still resolves and is still accepted, but has no enum member: one canonical spelling per option is most of the point of having an enum. -## Deprecation timetable for plain strings +## Both spellings are permanent -| Version | What happens | -| --- | --- | -| 0.4.0 | Enums added. Strings accepted, no warning. | -| **0.5.0 (current)** | Strings emit `DeprecationWarning`. | -| 1.0.0 | Strings removed; the enums become the only accepted form. | - -0.4.0 deliberately warned about nothing, because `mode="batch"` was what this documentation showed -until that release. The written notice came first, and this is the warning that follows it. - -The warning names the exact replacement, so migrating is a substitution you read off the message: - -``` -DeprecationWarning: Passing mode='batch' as a plain string is deprecated and will -stop working in 1.0.0. Use TrainingMode.BATCH instead. -``` +Neither form is deprecated and neither is going away. -Two things it deliberately does not do. It does not fire for a spelling that was never valid, so -`mode="stochastic"` still raises its `ValueError` rather than burying the real mistake under a -notice about modernising something that never worked. And it does not fire when a map is loaded -from an artifact: the name in a `.npz` is a serialisation detail, since JSON has no enums, so the -loader converts it rather than warning you about a string you never wrote. +0.5.0 briefly made plain strings emit `DeprecationWarning`, announcing removal in 1.0.0. **0.6.0 +withdrew that.** If you migrated in between, your code still works and nothing is wasted beyond the +time; the enums are staying too. -To silence it while you migrate: +The reason for the reversal is that every comparable library passes options as strings: -```python -import warnings - -warnings.filterwarnings("ignore", category=DeprecationWarning, module="python_som") -``` - -Migrating early costs nothing and is a mechanical substitution: `"batch"` becomes -`TrainingMode.BATCH`, and so on down the table above. +| library | how options are passed | +| --- | --- | +| scikit-learn | `KMeans(init="k-means++", algorithm="lloyd")` | +| numpy | `np.pad(mode="constant")`, `np.linalg.norm(ord="fro")` | +| scipy | `linkage(method="single")`, `minimize(method="BFGS")` | +| minisom | `neighborhood_function="gaussian"`, `topology="rectangular"` | +| sompy | `normalization="var"`, `neighborhood="gaussian"` | + +None of them export enums at all. Removing the string form would have made this the only library in +its ecosystem to reject `mode="batch"`, a worse trade than any tidiness it bought. + +The argument originally made for enums was that a type checker catches typos. That benefit is real +and already delivered by the `Literal` unions above, without removing anything: `mode="bacth"` is a +type error today while `mode="batch"` is not. ## Learning rate diff --git a/src/python_som/_enums.py b/src/python_som/_enums.py index 03655af..2da5803 100644 --- a/src/python_som/_enums.py +++ b/src/python_som/_enums.py @@ -4,10 +4,16 @@ 0.5.x series. ``mode=TrainingMode.BATCH`` and ``mode="batch"`` are interchangeable, compare equal, hash equal, and serialise to the same JSON, because each member *is* a ``str``. -**Deprecation runway.** Plain strings are deprecated as of 0.4.0 and emit ``DeprecationWarning`` -from 0.5.0. 0.4.0 deliberately warned about nothing, because ``mode="batch"`` was what every -documentation page showed at the time, so the written notice came first and this is the warning that -follows it. 1.0.0 removes strings entirely, and :func:`warn_if_string` goes with them. +**Both spellings are permanent.** 0.5.0 briefly deprecated plain strings and 0.6.0 withdrew that, +because every comparable library passes options as strings: scikit-learn +(``KMeans(init="k-means++")``), numpy (``np.pad(mode="constant")``), scipy +(``linkage(method="single")``), and both SOM peers, minisom and sompy. None of them export enums at +all. Being the only library in the ecosystem to reject ``mode="batch"`` would cost users more than +the consistency was worth. + +The enums remain because they cost nothing and some callers prefer them. The type-checking benefit +that motivated them is delivered by the ``Literal`` unions below rather than by removing anything: +``mode="bacth"`` is a type error while ``mode="batch"`` is not. **On the base class.** ``enum.StrEnum`` arrived in Python 3.11 and this package supports 3.10, so :class:`_StrEnum` reproduces it. A bare ``class X(str, Enum)`` is *not* equivalent: its ``str()`` @@ -18,7 +24,6 @@ from __future__ import annotations -import warnings from enum import Enum from typing import Literal @@ -31,7 +36,6 @@ "TrainingModeStr", "WeightInit", "WeightInitStr", - "warn_if_string", ] @@ -110,48 +114,3 @@ class SampleMode(_StrEnum): #: Accepted strings for the ``sample_mode`` argument of random initialization. SampleModeStr = Literal["standard_normal", "uniform"] - - -#: Names that are accepted but have no member of their own, mapped to the member that replaces them. -#: ``mexicanhat`` is the spelling the original contribution used; the enum carries one canonical -#: spelling per option, so this is where the other one is answered for. -_LEGACY_SPELLINGS = {"mexicanhat": "Neighborhood.MEXICAN_HAT"} - - -def warn_if_string(value: object, enum: type[Enum], parameter: str, *, stacklevel: int = 3) -> None: - """Emit a ``DeprecationWarning`` when a plain string is passed instead of an enum member. - - An enum member *is* a ``str``, so the test is "a string that is not one of ours" rather than - ``isinstance(value, str)``, which would fire on the enum too. - - Only *valid* spellings warn. A string naming nothing returns silently and is left to whatever - validates it, so ``mode="stochastic"`` still raises its ``ValueError`` rather than surfacing a - deprecation notice about a spelling that never worked. - - The message names the exact replacement rather than describing one. A deprecation warning that - makes the reader work out the substitution is a deprecation warning people silence. - - :param value: What the caller passed. - :param enum: The enum that should replace a string here. - :param parameter: Name of the parameter, for the message. - :param stacklevel: Frames to skip so the warning blames the caller's line rather than this one. - """ - if isinstance(value, enum) or not isinstance(value, str): - return - - try: - replacement = f"{enum.__name__}.{enum(value).name}" - except ValueError: - replacement = _LEGACY_SPELLINGS.get(value, "") - if not replacement: - # Not a value this enum knows and not a legacy alias, so it is simply wrong. Whoever - # validates it will say so; telling the caller to modernise a spelling that was never - # valid would bury the actual error under a deprecation notice. - return - - warnings.warn( - f"Passing {parameter}={value!r} as a plain string is deprecated and will stop working in " - f"1.0.0. Use {replacement} instead.", - DeprecationWarning, - stacklevel=stacklevel, - ) diff --git a/src/python_som/_som.py b/src/python_som/_som.py index 98ba5ad..5017f78 100644 --- a/src/python_som/_som.py +++ b/src/python_som/_som.py @@ -52,7 +52,6 @@ TrainingModeStr, WeightInit, WeightInitStr, - warn_if_string, ) from ._version import __version__ @@ -272,7 +271,6 @@ def __init__( self._min_neighborhood_radius = float(min_neighborhood_radius) self._neighborhood_function_name = neighborhood_function self._neighborhood_function: NeighborhoodFunction = resolve(neighborhood_function) - warn_if_string(neighborhood_function, Neighborhood, "neighborhood_function") self._distance_function = distance_function self._cyclic = (bool(cyclic_x), bool(cyclic_y)) @@ -442,7 +440,6 @@ def train( f"Value should be one of {list(TRAINING_MODES)}" ) raise ValueError(msg) - warn_if_string(mode, TrainingMode, "mode") if mode == "batch" and self._neighborhood_function_name in SIGNED_NEIGHBORHOODS: msg = ( f"The {str(self._neighborhood_function_name)!r} neighborhood function cannot be " @@ -756,9 +753,6 @@ def weight_initialization( f"Value should be one of {list(INITIALIZATION_MODES)}" ) raise ValueError(msg) - warn_if_string(mode, WeightInit, "mode") - if "sample_mode" in kwargs: - warn_if_string(kwargs["sample_mode"], SampleMode, "sample_mode") try: if mode == "random": self._weights = random_models( diff --git a/tests/test_typed_seams.py b/tests/test_typed_seams.py index e369588..8a1604a 100644 --- a/tests/test_typed_seams.py +++ b/tests/test_typed_seams.py @@ -8,7 +8,6 @@ from __future__ import annotations import json -import re import warnings from typing import TYPE_CHECKING @@ -94,9 +93,9 @@ def test_every_neighborhood_member_resolves() -> None: # --------------------------------------------------------------------------------------------- # Enums and strings are interchangeable at every call site that takes one # -# From 0.5.0 a plain string also warns, so each of these asserts both halves: the string still does -# exactly what the enum does, *and* it says it is going away. The warning is the point of the -# release, so it is asserted rather than filtered out. +# Both spellings are permanent, so these assert only that they agree. 0.5.0 briefly made the string +# form warn and each of these wrapped the string call in `pytest.warns`; 0.6.0 withdrew that, so the +# wrappers are gone rather than loosened. # --------------------------------------------------------------------------------------------- @@ -115,8 +114,7 @@ def test_training_accepts_either_spelling_with_identical_results( first = make_som(x=6, y=5) second = make_som(x=6, y=5) 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) # type: ignore[arg-type] + error_string = second.train(blobs, n_iteration=15, mode=as_string) # type: ignore[arg-type] assert error_enum == error_string np.testing.assert_array_equal(first.get_weights(), second.get_weights()) @@ -132,22 +130,16 @@ def test_weight_initialization_accepts_either_spelling( from_enum = som.get_weights().copy() other = make_som(x=5, y=4) - with pytest.warns(DeprecationWarning, match=f"mode={member.value!r}"): - other.weight_initialization(mode=member.value, **kwargs) + other.weight_initialization(mode=member.value, **kwargs) np.testing.assert_array_equal(from_enum, other.get_weights()) @pytest.mark.parametrize("member", list(Neighborhood)) def test_the_constructor_accepts_either_spelling(member: Neighborhood) -> None: from_enum = python_som.SOM(x=4, y=4, input_len=3, neighborhood_function=member, random_seed=1) - with pytest.warns(DeprecationWarning, match=f"neighborhood_function={member.value!r}"): - from_string = python_som.SOM( - x=4, - y=4, - input_len=3, - neighborhood_function=member.value, - random_seed=1, - ) + from_string = python_som.SOM( + x=4, y=4, input_len=3, neighborhood_function=member.value, random_seed=1 + ) np.testing.assert_array_equal( from_enum.neighborhood((2, 2), 1.5), from_string.neighborhood((2, 2), 1.5) ) @@ -158,8 +150,7 @@ def test_sample_mode_accepts_either_spelling() -> None: first = make_som(x=4, y=4) first.weight_initialization(mode=WeightInit.RANDOM, sample_mode=mode) second = make_som(x=4, y=4) - with pytest.warns(DeprecationWarning, match=f"sample_mode={mode.value!r}"): - second.weight_initialization(mode=WeightInit.RANDOM, sample_mode=mode.value) + second.weight_initialization(mode=WeightInit.RANDOM, sample_mode=mode.value) np.testing.assert_array_equal(first.get_weights(), second.get_weights()) @@ -257,117 +248,77 @@ def test_the_warning_points_at_the_caller() -> None: # --------------------------------------------------------------------------------------------- -# The deprecation itself +# No deprecation: strings are permanent # --------------------------------------------------------------------------------------------- -#: Each valid plain-string spelling, with the replacement its warning must name. -DEPRECATED_SPELLINGS = [ - ( - "train-mode", - lambda som, data: som.train(data, n_iteration=5, mode="batch"), - "TrainingMode.BATCH", - ), - ( - "init-mode", - lambda som, data: som.weight_initialization(mode="linear", data=data), - "WeightInit.LINEAR", - ), - ( - "sample-mode", - lambda som, _data: som.weight_initialization(mode=WeightInit.RANDOM, sample_mode="uniform"), - "SampleMode.UNIFORM", - ), -] - -#: Spellings that never named anything. These must raise, not warn. -INVALID_SPELLINGS = [ - ("bad-train-mode", lambda som, data: som.train(data, n_iteration=1, mode="stochastic")), - ("bad-init-mode", lambda som, _data: som.weight_initialization(mode="spectral")), - ( - "bad-sample-mode", - lambda som, _data: som.weight_initialization(mode=WeightInit.RANDOM, sample_mode="cauchy"), - ), -] - @pytest.mark.parametrize( - ("call", "expected"), - [(c, e) for _, c, e in DEPRECATED_SPELLINGS], - ids=[i for i, _, _ in DEPRECATED_SPELLINGS], + "call", + [ + lambda som, data: som.train(data, n_iteration=5, mode="batch"), + lambda som, data: som.weight_initialization(mode="linear", data=data), + lambda som, _d: som.weight_initialization(mode=WeightInit.RANDOM, sample_mode="uniform"), + lambda som, _d: som.weight_initialization(mode="random"), + ], + ids=["train-mode", "init-mode", "sample-mode", "init-random"], ) -def test_the_warning_names_the_exact_replacement( - call: Callable[[python_som.SOM, np.ndarray], object], expected: str +def test_a_plain_string_emits_no_warning( + call: Callable[[python_som.SOM, np.ndarray], object], ) -> None: - """A deprecation that makes the reader work out the substitution is one they will silence.""" + """0.5.0 made these warn and 0.6.0 withdrew it. This is the guard for that withdrawal. + + Every comparable library takes options as strings -- scikit-learn, numpy, scipy, and both SOM + peers -- and none export enums. Deprecating the string form would have made this the only + library in its ecosystem to reject ``mode="batch"``. + + ``simplefilter("error")`` rather than ``pytest.warns(None)``, which pytest removed: any warning + at all fails here, not merely a ``DeprecationWarning``. + """ som = make_som(x=5, y=4) data = np.random.default_rng(1).normal(size=(20, 3)) - with pytest.warns(DeprecationWarning, match=re.escape(expected)): + with warnings.catch_warnings(): + warnings.simplefilter("error") call(som, data) -def test_the_legacy_neighborhood_spelling_warns_toward_the_canonical_member() -> None: - """``mexicanhat`` has no member of its own, so the message names the one that replaces it.""" - with pytest.warns(DeprecationWarning, match=re.escape("Neighborhood.MEXICAN_HAT")): - python_som.SOM( +def test_the_legacy_neighborhood_spelling_still_works_silently() -> None: + """``mexicanhat`` predates the ``mexican_hat`` spelling and keeps working, without complaint.""" + with warnings.catch_warnings(): + warnings.simplefilter("error") + som = python_som.SOM( x=4, y=4, input_len=3, neighborhood_function="mexicanhat", ) + assert som.neighborhood((2, 2), 1.0).shape == (4, 4) @pytest.mark.parametrize( - "call", [c for _, c in INVALID_SPELLINGS], ids=[i for i, _ in INVALID_SPELLINGS] + "call", + [ + lambda som, data: som.train(data, n_iteration=1, mode="stochastic"), + lambda som, _d: som.weight_initialization(mode="spectral"), + lambda som, _d: som.weight_initialization(mode=WeightInit.RANDOM, sample_mode="cauchy"), + ], + ids=["bad-train-mode", "bad-init-mode", "bad-sample-mode"], ) -def test_an_invalid_string_raises_rather_than_warning( +def test_an_invalid_string_still_raises( call: Callable[[python_som.SOM, np.ndarray], object], ) -> None: - """A spelling that never worked is an error, not a deprecation. - - Warning would bury the real mistake under a notice telling the caller to modernise something - that was never valid, and under ``-W error`` would replace the ``ValueError`` outright. - """ + """Supporting strings is not the same as accepting any string.""" som = make_som(x=5, y=4) data = np.random.default_rng(1).normal(size=(20, 3)) - with warnings.catch_warnings(): - warnings.simplefilter("error") - with pytest.raises(ValueError, match=r"Invalid value|sample_mode"): - call(som, data) - - -def test_an_enum_never_warns() -> None: - """The whole point: migrating removes the warning.""" - data = np.random.default_rng(2).normal(size=(30, 3)) - with warnings.catch_warnings(): - warnings.simplefilter("error") - som = python_som.SOM( - x=5, y=4, input_len=3, neighborhood_function=Neighborhood.GAUSSIAN, random_seed=3 - ) - som.weight_initialization(mode=WeightInit.RANDOM, sample_mode=SampleMode.UNIFORM) - som.train(data, n_iteration=5, mode=TrainingMode.BATCH) - - -def test_the_deprecation_warning_blames_the_caller() -> None: - """``stacklevel`` must point at the user's line, not at a line inside this package.""" - som = make_som(x=4, y=4) - with pytest.warns(DeprecationWarning, match="deprecated") as caught: - som.weight_initialization(mode="random") - assert caught[0].filename == __file__, f"warning blamed {caught[0].filename}" + with pytest.raises(ValueError, match=r"Invalid value|sample_mode"): + call(som, data) def test_error_messages_read_the_same_for_both_spellings() -> None: - """An enum member's repr is ````, which has no place in an error. - - Regression: the messages interpolated the value directly, so passing an enum produced - `` initialization requires ...``. - """ + """An enum's repr is ````, which does not belong in an error.""" som = make_som(x=4, y=4) with pytest.raises(ValueError, match="initialization requires") as from_enum: som.weight_initialization(mode=WeightInit.LINEAR) - with ( - pytest.warns(DeprecationWarning, match="deprecated"), - pytest.raises(ValueError, match="initialization requires") as from_string, - ): + with pytest.raises(ValueError, match="initialization requires") as from_string: som.weight_initialization(mode="linear") assert str(from_enum.value) == str(from_string.value) assert "WeightInit." not in str(from_enum.value)