Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plum 2 #68

Closed
wesselb opened this issue Oct 23, 2022 · 10 comments · Fixed by #73
Closed

Plum 2 #68

wesselb opened this issue Oct 23, 2022 · 10 comments · Fixed by #73

Comments

@wesselb
Copy link
Member

wesselb commented Oct 23, 2022

What is This?

This issue proposes possible major improvements for a version two of the package. The goal is to start a discussion around these proposals. If you agree or disagree with some or all of changes, or think that other improvements are missing, then please do share your thoughts by commenting below. :) Tagging @PhilipVinc, @tbsexton, @leycec, and @seeM here since they have been involved in recent relevant discussions.

None of the proposed changes should change current behaviour, thus maintaining backwards compatibility. I'll be continuously updating and adding to this issue over time.

The list of proposed improvements is as follows:

Instance-Check-Based Dispatch

Instead of performing dispatch based on the types of the arguments, perform dispatch entirely using isinstance, hence fully eliminating the need to use type_of. See this proposal by @tbsexton. The massive benefit of doing things this way is it becomes much easier to support types like Literals (#66), Protocols (#28), numpy arrays with a specific number of dimensions (#10), and PyTrees (Stheno #21).

The big downside of this change is that performance will take a hit, since instance checks will need to happen every time dispatch happens. However, by using @beartype (@leycec), instance checks will be blazingly fast, so the performance hit should be minimal. Moreover, caching is still possible for functions with a so-called faithful dispatch tree (see here), so we should get the best of both worlds!

Although dispatch will not rely on type_of anymore, other functionality of the package still does, such as promotion. Hence, a type_of function is still necessary.

Postponed Evaluation of Types

@beartype now includes beartype.peps.resolve_pep538(), which should handle PEP538-style postponed types for us. A massive thanks to @leycec for this!! This allows us to get rid of all use of inspect, which I believe currently is causing performance issues (Stheno #25). Deferring the responsibility of handling postponed evaluation of types to @beartype should significantly simplify the internals of the package.

Wrapping of Types

Internally Plum currently wraps types in its own types. We should get rid of those entirely and use types from typing. This does pose a challenge for some more advanced custom types, such as PromisedType.

Compatibility with Modern Tooling

Whatever Plum does should be compatible with modern tooling. Think mypy, pylance (#51), and possible others.

@rtbs-dev
Copy link
Contributor

Awesome! I'll have to go through a bit more later

As for promotion, isn't that just a baked-in dispatched function? I was checking the code for hard dependencies on a type_of kind of functionality, but at first glance it looks like it's a dispatched function anyway, which gets a list of "known" promotion rules.

So theoretically, all of the tree search stuff should still apply? Plum provides some decent default promotions, which the underlying dispatcher can do a lookup on, and users can extend with their own types.

We don't expect a situation where the promotion function should know what to do with a type that neither the devs or the users have told it about... right?

@wesselb
Copy link
Member Author

wesselb commented Oct 24, 2022

As for promotion, isn't that just a baked-in dispatched function?

So theoretically, all of the tree search stuff should still apply?

Yes, I would agree! My comment in the initial post is more a practical one. Namely, currently the code relies on a function type_of. Most likely it can be refactored to eliminate all uses of type_of, but I guess that I'm not 100% certain that we can do this until we actually do it.

We don't expect a situation where the promotion function should know what to do with a type that neither the devs or the users have told it about... right?

Yeah, I think you're right!

@leycec
Copy link
Member

leycec commented Oct 26, 2022

Brilliance personified! As an addendum to #53, I hereby promise that beartype 0.13.0 (...so, the next @beartype after the @beartype I hope to publish this weekend) will implement deep type-checking for containers everyone cares about – including dictionaries and sets.

I'll try to remember to post an update here when beartype 0.13.0 lands. Alternately, we're also tracking progress on deep type-checking at beartype/beartype#167 and beartype/beartype#53. Please throw tomatoes at me there if Plum desperately needs something currently omitted by @beartype.

We now return to your regularly scheduled brilliance. I'm so impressed my mouth is agape. 😮

@wesselb
Copy link
Member Author

wesselb commented Oct 30, 2022

Oh hell yeah, that would be absolutely fantastic! As always, @leycec, thank you so much for your time and effort spent on @beartype. @beartype is making everyone's lifes nicer. :)

Please throw tomatoes at me there if Plum desperately needs something currently omitted by https://github.com/beartype.

Will do! 🍅

@astanziola
Copy link

If I may add something to the whishlist of v2.0 of this amazing project, would be to be able to use automated documentaton (either via sphinx or mkdocstrings).

I have been experimenting a bit bit haven't found a way that doesn't seem like an hack (and maybe there is not one?)

@wesselb
Copy link
Member Author

wesselb commented Jan 24, 2023

@astanziola I fully agree that making multiple function definitions play nicely with automated documentation is a very necessary feature that's been lacking for a long time. I currently have something in the works that functions in the following way.

Consider the following file:

from numbers import Number

from plum import dispatch


@dispatch.abstract
def f(x: Number, y: Number):
    """Multiply two numbers."""


@dispatch
def f(x: float, y: float):
    """Multiply two floats."""
    return x * y


@dispatch
def f(x: int, y: int):
    """Multiply two ints."""
    return x * y

Then calling help(f) produces

Help on Function in module __main__:

f(x: numbers.Number, y: numbers.Number)
    Multiply two numbers.

    This function has further implementations documented below.

    f(x: float, y: float)
        Multiply two floats.

    f(x: int, y: int)
        Multiply two ints.

The result for automated documentation would be similar. I agree that this isn't ideal, because the other function definitions are embedded as text in the docstring, but at least all methods appear in the documentation. A much nicer solution, of course, would be to write a plugin for Sphinx that produces a separate entry for every method of a function. Baby steps first I suppose 😅

@astanziola
Copy link

Yes, and probably it makes more sense to develop it separately from plum.

At some point I wrote this to work with mkdocs and plum and at least shows the methods, but is one piece of code that I'm particoularly not proud of (it is really, really hacky) and I don't know much about the bits underlying automatic documentation to do it properly.

Anyhow, this is probably not the best place to talk about this, I just wanted to raise the issue :)
Thanks again for this great project!

@rtbs-dev
Copy link
Contributor

Thought I'd add my 2c here... I'm pretty sure this might be somewhat addressed by the various docstring groups already, via @overload. It's a "do-nothing" decorator, but shows mypy that a function can have multiple type-signatures.

I think, if Plum 2 can somehow automatically imply that calling dispatch also implies a call to @overload, then sphinx and mkdocs both have other solutions already for documentation.

@leycec
Copy link
Member

leycec commented Jan 25, 2023

@beartype crew: combine all your powers!

There are three API generators of general interest in Python. Technically, there are more (e.g., Twisted's pydoctor). Pragmatically, nobody cares about those. What's left is:

  • Sphinx's bundled autodoc extension. Since both Plum and autodoc are runtime-oriented, Plum should already work out-of-the-box with autodoc. Courtesy official autodoc documentation:

If you document decorated functions or methods, keep in mind that autodoc retrieves its docstrings by importing the module and inspecting the __doc__ attribute of the given function or method. That means that if a decorator replaces the decorated function with another, it must copy the original __doc__ to the new function.

  • Sphinx's third-party autoapi extension, maintained by Read The Docs (RTD). Since RTD does it and since autodoc ostensibly suuuuucks, everyone and their pet chimera cat-dogs are migrating from autodoc to autoapi. Unlike autodoc, autoapi only performs static analysis. Problems for Plum start here.
  • mkdoc's mkdocstrings project. Like autoapi, mkdocstrings only performs static analysis. Problems for Plum continue here.

autoapi and mkdocstrings are two pain points possibly warranting Plum-specific plugins. But nobody wants to write, debug, document, and maintain those. Instead, we randomly try the standard typing-centric workaround that autoapi and mkdocstrings will absolutely ignore – because we're on that timeline:

from typing import TYPE_CHECKING, overload

# If somebody is statically analyzing us, reduce Plum's
# @dispatch decorator to the standard @overload decorator.
if TYPE_CHECKING:
    dispatch = overload
# Else, somebody is running us at runtime. In this case,
# define Plum's @dispatch decorator in the standard way.
else:
    from collections.abc import Callable
    from typing import TypeVar
    T = TypeVar('T', bound=Callable)

    def dispatch(T) -> T: ...

If either autoapi or mkdocstrings do not currently support typing.TYPE_CHECKING, they probably should. I consider that an upstream bug. Upstream should be notified. Blame upstream! I voted for Plum.

@wesselb wesselb mentioned this issue Feb 20, 2023
@wesselb
Copy link
Member Author

wesselb commented Mar 3, 2023

@astanziola

At some point I wrote this to work with mkdocs and plum and at least shows the methods, but is one piece of code that I'm particoularly not proud of (it is really, really hacky) and I don't know much about the bits underlying automatic documentation to do it properly.

Ahhh, this is nice! Even though it is hacky, the result looks nice and it would be exactly what we're looking for.

Anyhow, this is probably not the best place to talk about this, I just wanted to raise the issue :) Thanks again for this great project!

Thanks to you for raising the issue! :) You're totally right, and IMO this is something that should be addressed on the short term, if at all possible.


@tbsexton @leycec

autoapi and mkdocstrings are two pain points possibly warranting Plum-specific plugins. But nobody wants to write, debug, document, and maintain those. Instead, we randomly try the standard typing-centric workaround that autoapi and mkdocstrings will absolutely ignore – because we're on that timeline: <snipped snippet>

Ahhh, this is very clever! I've yet to try this, but when I get around to freeing up some time to more closely look into the documentation isssue, this will be the first item on my to-try list. :) It would be very nice if there were a solution that doesn't require writing plugins.

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 a pull request may close this issue.

4 participants