Skip to content

Add Version.diff classification matching node-semver 7.7.2 - #493

Open
masonjian wants to merge 2 commits into
python-semver:masterfrom
masonjian:feat/version-diff-262
Open

masonjian wants to merge 2 commits into
python-semver:masterfrom
masonjian:feat/version-diff-262

Conversation

@masonjian

@masonjian masonjian commented Oct 11, 2026 •

Copy link
Copy Markdown

Related to #262. This follows the maintainer-confirmed scope and does not fully resolve the issue.

Add Version.diff(other) classification matching node-semver 7.7.2 for valid version pairs:

  • Return major, minor, patch, premajor, preminor, prepatch, or prerelease. Equal precedence returns None; build metadata is ignored.
  • Normalize higher/lower versions so swapping operands gives the same result, including node-semver's special prerelease-to-release cases.
  • Accept Version, str, bytes, dict, tuple, and list, with tests for unsupported inputs and invalid versions. Existing Version subclasses are not reconstructed.
  • Add usage documentation, a changelog entry, and ISC attribution.

This is classification only. next_version, bump behavior, and staging policy are unchanged.

Validation on macOS with Python 3.12.13:

  • All 76 diff tests pass; 50 failed against the original implementation.
  • Full pytest suite, including doctests: 564 passed, 1 skipped. Source statement and branch coverage: 100%.
  • All 36 upstream diff cases pass in both directions; 72,900 ordered pairs match the actual node-semver 7.7.2 implementation.
  • Strict Sphinx build, Ruff and formatting for the new diff tests, and git diff --check pass.
  • Repository-wide Ruff reports 160 pre-existing findings. Strict mypy remains blocked by the missing optional backend; comparison against the original HEAD found no new findings.

The full tox multi-Python/OS matrix and optional native backend were not run locally. At the latest check, GitHub's Python and CodeQL workflows awaited maintainer approval.

Independent review found no blocking issues. AI assistance was used for implementation, automated testing, and review.

@masonjian
masonjian marked this pull request as ready for review October 11, 2026 01:52

@tomschr tomschr left a comment •

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @masonjian, thanks for picking up #262! Really appreciate your contribution! ❤️ The tests and docs are thorough.

Unfortunately, I don't think this PR addresses the original issue. The motivating example is choosing a bump on a staging branch. If staging is 1.0.1-rc.1 and master is 1.0.1, the answer should be bump_minor(). Here, Version.parse("1.0.1-rc.1").diff(Version.parse("1.0.1")) returns 'prerelease', which would point users to bump_prerelease().

I'd like to propose a different direction: make diff match node-semver's diff. Node's semver.diff (checked against semver@7.7.2) differs in ways people will likely notice:

v1 → v2 node semver.diff This PR
1.2.3+one → 1.2.3+two null build
1.2.3-alpha → 1.2.3 patch prerelease
1.2.3 → 1.2.4-rc.1 prepatch patch

Node returns a bump type with a pre prefix for prerelease targets, ignores build metadata, and is direction-aware.

Python users who know node-semver would then get the same answers, and the function would return a bump type rather than a field name. Node's algorithm ports cleanly:

def diff(self, other: Comparable) -> str | None:
    # operand coercion as in the current version

    cmp = self.compare(other)
    if cmp == 0:
        return None

    high, low = (self, other) if cmp > 0 else (other, self)
    high_pre = bool(high.prerelease)
    low_pre = bool(low.prerelease)

    if low_pre and not high_pre:
        if not low.minor and not low.patch:
            return "major"
        if low.to_tuple()[:3] == high.to_tuple()[:3]:
            return "minor" if low.minor and not low.patch else "patch"

    prefix = "pre" if high_pre else ""
    for part in ("major", "minor", "patch"):
        if getattr(self, part) != getattr(other, part):
            return prefix + part
    return "prerelease"

What this changes:

  • Build-only differences return None, because compare() ignores build metadata. This matches node and ==, so the 'build' return value goes away.
  • The result depends on which version is higher, matching node's behavior.
  • The return values are major, minor, patch, premajor, preminor, prepatch, prerelease, or None.
    Node's quirks come along. For example, 1.0.0-alpha → 1.0.0 returns major, and 1.0.1-rc.1 → 1.0.1 returns patch.

This still doesn't solve the staging case from #262, since node returns patch there too. If you want that answer, it would need a separate, explicitly named method, or docs showing how to combine diff with a core comparison.

This is a rewrite of the method and its tests rather than a patch. The tests could be based on node-semver's own diff test cases, which would give good coverage and a clear parity check. The docs would also need to change from "first differing part" to "bump type, matching node-semver".

Happy to review a revision, and it may be worth agreeing on the direction in #262 first.

Comment thread docs/usage/compare-versions.rst
Comment thread docs/usage/compare-versions.rst Outdated
Comment thread src/semver/version.py Outdated
Comment thread tests/test_diff.py Outdated


@pytest.mark.parametrize(
"other", [None, 1, "1.2.3", b"1.2.3", (1, 2, 3), [1, 2, 3], {"major": 1}]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test currently asserts that "1.2.3", b"1.2.3", (1, 2, 3), [1, 2, 3], and {"major": 1} raise TypeError. Those should now be accepted, so move them into a positive test. Keep None, 1, and similar invalid types in the rejection test.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved the supported input types into positive conversion tests in b89678f, including full prerelease/build fields and incomplete collection inputs. The rejection test now keeps None, 1, and other unsupported types.

Comment thread tests/test_diff.py
Comment thread changelog.d/262.feature.rst Outdated
Comment on lines +1 to +3
Added ``Version.diff(other)`` to identify the first differing version part,
including build metadata, independently of comparison direction. It returns
``None`` when all parts are identical.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we want to follow my suggestion, we would need to adjust the wording:

Suggested change
Added ``Version.diff(other)`` to identify the first differing version part,
including build metadata, independently of comparison direction. It returns
``None`` when all parts are identical.
Added :meth:`~semver.version.Version.diff` to return the type of bump needed to go from one version to another, matching the ``semver.diff`` function in node-semver. It returns ``"major"``, ``"minor"``, ``"patch"``, ``"premajor"``, ``"preminor"``, ``"prepatch"``, or ``"prerelease"``, or ``None`` when the versions have the same precedence. Build metadata is ignored.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the changelog in b89678f with all seven node-semver 7.7.2 classifications, None for equal precedence, and ignored build metadata. I used "classify valid version pairs" and explicitly noted that staging-branch bump policy is outside this change, following the scope agreed in #262.

Comment thread docs/usage/compare-versions.rst Outdated
@masonjian masonjian changed the title Add Version.diff to identify the first differing version part Add Version.diff classification matching node-semver 7.7.2 Oct 11, 2026

This branch has not been deployed

No deployments
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.

2 participants