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)