Skip to content

Remove the compare() -> _compare() deprecation shim #215

Description

@sromoam

Summary

#209 renames the comparator extension point from compare() to _compare(), and ships a deprecation shim so existing out-of-tree comparators keep working. This issue tracks removing that shim in 0.8.0.

Filing this at the same time as the shim lands, deliberately. A deprecation without a scheduled removal becomes permanent.

Background

BaseComparator.compare() is now a template method holding the shared None policy, and subclasses implement _compare(). Comparators written against the pre-0.7 interface implement compare() directly.

Rather than break them, BaseComparator.__init_subclass__ leaves such a class exactly as written, fills the _compare slot so the class is not abstract, and emits a DeprecationWarning naming the rename. The customer's comparator keeps working with unchanged behavior.

The tradeoff, and the reason this cannot be permanent: an un-migrated comparator does not get the None policy. Its compare() shadows the template method, so (None, None), (None, "") and similar are handled by whatever the author wrote. That is precisely the divergence #200 set out to eliminate, so the shim buys migration time at the cost of leaving the original bug reachable in user code.

What to remove in 0.8.0

  1. BaseComparator.__init_subclass__ in src/stickler/comparators/base.py, along with the _unmigrated stub and the warn_once call.
  2. The warn_once import if nothing else in that module uses it.
  3. The migration-surface tests in tests/common/comparators/test_none_handling.py that assert legacy shapes still work (TestPreRenameSubclassMigration and the shim-specific cases), replaced with tests asserting they now fail.
  4. The shim's mention in src/stickler/comparators/Comparators.md and docs/docs/Guides/Comparators/README.md.

After removal, a comparator implementing only compare() and extending BaseComparator directly becomes abstract again and raises TypeError at construction. A comparator extending a concrete comparator and overriding compare() still constructs and still bypasses the policy: that is plain Python MRO, not something the shim causes, and it cannot be fixed by removing the shim. If we want that shape to fail loudly, it needs its own __init_subclass__ guard that raises, which is a separate decision.

Migration cost for users

Two lines per comparator:

# before
def compare(self, a, b):
    if a is None or b is None:
        return 0.0
    return ...

# after
def _compare(self, a, b):
    return ...

#209 makes exactly this change to the repo's own two CaseInsensitiveComparator classes without altering any test expectation.

Before removing

  • Confirm the deprecation has been in at least one released version with the warning visible, so users have had a real window rather than a technically-announced one.
  • Check the CHANGELOG entry from fix: handle None consistently across all comparators #209 names the removal, and update it here if the target version moves.
  • Consider whether the release notes should include a short migration snippet rather than only a prose description.

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type

    Projects

    Status
    Backlog

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions