Feature: API diffing utils & change detection.

[tlambert03]

Continuing conversation started here ...

One of the things I'm most interested in using griffe for is detecting API changes. This is one of the main things mentioned in the README Todo, so is clearly a major interest to @pawamoy as well. There's a lot of subtleties to discuss here, so just starting this issue as courtesy/placeholder for this topic.

I've started an experimental repo to play with this here: https://github.com/tlambert03/griffediff ... but would be happy to port that work into this repo later if it's decided that that is the best place for it. (or, alternatively, it could also be an independent repo, but still live in the mkdocstrings org). There's lots of discussion to be had about approach there too.

I'll also mention https://github.com/Carreau/frappuccino. I know that @Carreau there wanted to do something purely static/AST-based (which is what griffe would be doing particularly well here), but eventually went to dynamic inspection due to difficulties in actually being able to detect API changes statically. (for example, a simple decorator can completely change an AST node at runtime... so the actual scope/usefulness of static API detection is definitely an open question)

[pawamoy]

It's true that Python is (very) dynamic, and sometimes you have no choice but to actually execute things to get their actual representation, the one that users will see and use. But I think that a lot of projects can still be perfectly scanned with static analysis only. I myself very rarely use complex inheritance, decorations, re-binding/wrapping/re-assignments, etc. Most of the time my code is vanilla code. In some cases, visiting the AST is even necessary, for example to be able to pick up @overload-decorated methods (multiple signatures), which are simply overwritten when executing code. In any case, Griffe is able to both visit ASTs and introspect code (importing it), so I think it gives a solid foundation for API-diffing.

[pawamoy]

I've read your code @tlambert03 and I like the way you use git to provide a reusable way to diff between two commits. This is very library-friendly and extendable.

[tlambert03]

cool. Yeah, I'm hoping that will help in a CI context too. (i.e. run a github action and check the working tree against the main branch, and if a breaking change is detected, make sure there's a "!" in the PR title, or some other conventional commit marker of breaking change)

I do also think that it would be nice to compare/cache json artifacts, as you discuss in the README.

[pawamoy]

Hi @tlambert03, I hope you're doing well!

I'd like to push this feature (API diffing + warnings) forward. Are you still interested in working on it, be it in Griffe itself or in another project? I'd like to sync with you before starting anything 🙂

[tlambert03]

Hey @pawamoy, thanks for checking in. I haven't pushed any farther on this ... partially due to getting sidetracked with typing tools. In additional to pure function signature compatibility, I had hoped to also be able to check type compatibility (using type hints)... and worked with @leycec to add some conveniences to beartype which are now released in the beartype.door module as of v0.11.0... and might be useful here (though, of course, the need to actually have all forward references resolved there might pose some challenges for the static approach here).

I'm not sure exactly what angle you wanted to tackle this from, but I'm all ears to whatever you think is a good place to start! If you're curious to see where I was going with it, you could have a look through some of my tests in griffediff: https://github.com/tlambert03/griffediff/tree/main/tests ... but I'll emphasize that it was still very much in the experimentation phase!

I'm not sure how much time I have to devote to this in the near future, but suffice it to say I'm definitely interested in staying abreast of progress in this :) more than happy to see you push it forward here in griffe itself, and happy to contribute anything you think I can.

[leycec]

@beartype lead joins the chat. Thanks so much for pinging me on, @tlambert03! We should now mention that @tlambert03 single-handedly architected our entire beartype.door API via a gauntlet of gruesome PRs, including the pivotal beartype.door.is_subhint tester leveraged below for profit.

I got down to brass tacks and basically solved world peace via this minimal-length example defining a general-purpose is_func_api_preserved() tester. This tester leverages only public @beartype APIs to portably detect breakage in arbitrary callables via type hints alone in exactly ten lines of code: ^{...ignoring imports, docstrings, comments, and blank lines to make myself look better}

from beartype import beartype
from beartype.door import is_subhint
from beartype.peps import resolve_pep563
from collections.abc import Callable

@beartype
def is_func_api_preserved(func_new: Callable, func_old: Callable) -> bool:
    '''
    ``True`` only if the signature of the first passed callable (presumably the
    newest version of some callable to be published) preserves backward
    compatibility with the second passed callable (presumably an older version
    of the first passed callable) according to the PEP-compliant type hints
    annotating these two callables.

    Parameters
    ----------
    func_new: Callable
        Newest version of a callable to test for API breakage.
    func_old: Callable
        Older version of that same callable.

    Returns
    ----------
    bool
        ``True`` only if the signature of ``func_new`` preserves backward
        compatibility with the signature of ``func_old``.
    '''

    # Resolve all PEP 563-postponed type hints annotating these two callables
    # *BEFORE* reasoning with these type hints.
    resolve_pep563(func_new)
    resolve_pep563(func_old)

    # For the name of each annotated parameter (or "return" for an annotated
    # return) and the hint annotating that parameter or return for this newer
    # callable...
    for func_arg_name, func_new_hint in func_new.__annotations__.items():
        # Corresponding hint annotating this older callable if any or "None".
        func_old_hint = func_old.__annotations__.get(func_arg_name)

        # If no corresponding hint annotates this older callable, silently
        # continue to the next hint.
        if func_old_hint is None:
            continue
        # Else, a corresponding hint annotates this older callable.

        # If this older hint is *NOT* a subhint of this newer hint, this
        # parameter or return breaks backward compatibility.
        if not is_subhint(func_old_hint, func_new_hint):
            return False
        # Else, this older hint is a subhint of this newer hint. In this case,
        # this parameter or return preserves backward compatibility.

    # All annotated parameters and returns preserve backward compatibility.
    return True

The proof is in the real-world pudding:

>>> from numbers import Real

# New and successively older APIs of the same function.
>>> def new_func(text: str | None, ints: list[Real]) -> int: ...
>>> def old_func(text: str, ints: list[int]) -> bool: ...
>>> def older_func(text: str, ints: list) -> bool: ...

# Does the newest version of this function preserve backward
# compatibility with the next older version?
>>> is_func_api_preserved(new_func, old_func)
True  # <-- good. this is good.

# Does the newest version of this function preserve backward
# compatibility with the oldest version?
>>> is_func_api_preserved(new_func, older_func)
False  # <-- OH. MY. GODS.

In the latter case, the oldest version older_func() of that function ambiguously annotated its ints parameter to accept any list rather than merely a list of numbers. Both the newer version new_func() and the next older version old_func() resolve that ambiguity by annotating that parameter to accept only lists of numbers. Technically, that constitutes API breakage; users upgrading from the older version of the package providing older_func() to the newer version of the package providing new_func() could have been passing lists of non-numbers to older_func().

In the former case, new_func() relaxes the constraint from old_func() that this list contain only integers to accept a list containing both integers and floats. new_func() thus preserves backward compatibility with old_func().

This feature request ignited my interest away from things I probably should have been doing instead but which were alot less fun, like horrifying Sphinx documentation. @tlambert03 must feel similarly, because look at us – open-sourcing on Friday night like our keyboards depend on it. Gah!

Thus was Rome's API preserved in a day.

[pawamoy]

horrifying Sphinx documentation

Time to try MkDocs and mkdocstrings instead 😛? I think @posita managed to generate quite good looking docs with them, for numerary (hahaha, bringing all the fun people on this ticket).

---

Thanks for the very interesting example and explanation @leycec! In Griffe, we particularly need to show what is not compatible, and I guess your 10-lines snippet above can be adapted to do just that, easily 👍
The issue with all this, as @tlambert03 stated previously, is that is works at runtime only right? I mean, the resolve_pep563 function will dynamically import all the necessary types and classes, right? If that's how it works, I think it's an acceptable cost for such a nice feature in Griffe (API breaking change detection). Though I'd have to take a closer look at resolve_pep563 to see if it could integrate within Griffe's model.

[tlambert03]

Yeah, that's the tricky bit here. I'll be ~shocked if we can fully determine type compatibility without resorting to resolving forward refs, but if you can figure out a static substitute for python's issubclass, get_args, and get_origin using just the info derived from Griffe's member graph, we might be able to do something fun here 😅

Alternatively, we can have two levels of API comparison. One that simply looks at function argument names and positions, and one that goes a step farther to resolve hints and compare types

[pawamoy]

Yes, that's what I thought as well: first version doesn't care about types, and we add this later.

About static substitutes:

• issubclass: doable since we have the "base classes" data? That would require visiting the typing module(s) as well, probably.
• get_args and get_origin: seems hackish but doable as well thanks to Griffe's Expressions?

[pawamoy]

Just to update this thread: the version 0.24.0 of Griffe released this API breakage detection feature. It's a first version, it does not check types compatibility. I'll close this issue and open a new one for types compatibility. Thanks everyone for your help on this!