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

Hook for modifying method signatures #459

Open
jacobtomlinson opened this issue Jul 1, 2024 · 8 comments
Open

Hook for modifying method signatures #459

jacobtomlinson opened this issue Jul 1, 2024 · 8 comments

Comments

@jacobtomlinson
Copy link

Thanks for this plugin, it's really great!

I have a package which uses a class decorator to modify some methods of a class. Specifically it wraps coroutines in a sync wrapper.

When documenting with sphinx-autoapi the methods still show as async despite being replaced by a sync method by the decorator. I don't expect the parser to handle this, it's definitely an edge case, but I wondered if there was an event I could hook so I can write a plugin to fix this on my end?

@sachahu1
Copy link

sachahu1 commented Jul 1, 2024

@jacobtomlinson I've had similar cases happen and I wonder how "correct" that would be from a software point of view:

def decorator(func: Callable) -> Callable:
    pass

@decorator
async def async_func():
    pass

Technically the async_func is very much asynchronous, it just so happens that it is wrapped into another function which "converts" it. In a sense, if you "unpack" the decorator then you get:

def decorator(func: Callable) -> Callable:
    pass

async def async_func():
    pass

wrapped_func = decorator(async_func)

where async_func is very much async and wrapped_func is synchronous.

I hope that made sense. Like I said, I'm not sure how "correct" it would be to completely "ommit" async_func's own identity and overwrite it with the decorator's.

That being said, when I've had this kind of stuff happen, I've used python stub files to overwrite that. I believe it might work in this case?
(a simple stub file with)

def async_func(): ...

@sachahu1
Copy link

sachahu1 commented Jul 1, 2024

Follow up:
I've just tried adding a stub file to see how autoapi reacts. Technically works but you'd have to write a stub definition for all functions within the module so might not be ideal.

It could actually be nice to have a feature enhancement in autoapi to combine function definitions from both py and pyi.

@jacobtomlinson
Copy link
Author

jacobtomlinson commented Jul 1, 2024

Thanks for the quick responses!

My code does something similar to the following concept (but I've cut it down massively to make a simple example).

import asyncio
from functools import wraps
from typing import Callable


def decorator(func: Callable) -> Callable:
    @wraps(func)
    def inner(*args, **kwargs):
        asyncio.run(func(*args, **kwargs))
    return inner

@decorator
async def foo():
    await asyncio.sleep(1)
    print("Hello World!")

foo()

So in this case foo() is defined as a coroutine but then converted to a sync function because it is wrapped in inner() and then run via asyncio.run().

In my project it's slightly more complex because these are methods on classes, and the decorator is at the class level. There are over 40 classes, each with potentially tens of methods, so I'd prefer not to have to create a stub file and duplicate all of that structure.

Ideally I'd love to be able to just hook sphinx-autoapi and every time it parses a method quickly inspect whether the class was decorated and if so just remove the coroutine attribute from the method. For added complexity the decorator only wraps coroutine methods that aren't private and don't start with async_, so I would need to be able to filter those out.

@sachahu1
Copy link

sachahu1 commented Jul 1, 2024

@jacobtomlinson

Disclaimer: I am merely an autoapi enthusiast so I'll try to be as helpful as possible 😀

Right, that makes sense. I think there's still a question of whether completely overloading the original docstring makes sense. What I mean is that you're trying to overwrite your "decorated function's" docstring with the doctring from your inner function.

That being said, if that is still what you want, the following might help:

With more "traditional" sphinx doc, you'd probably do it like this:

import asyncio
from functools import wraps
from typing import Callable


def decorator(func: Callable) -> Callable:
    @wraps(func)
    def inner(*args, **kwargs):
    """A new docstring""""
        asyncio.run(func(*args, **kwargs))
    return inner

@decorator
async def foo():
    """A docstring (to be overloaded)""""
    await asyncio.sleep(1)
    print("Hello World!")

foo()

Obviously since sphinx-autoapi doesn't actually run any of the code this won't work here.

I don't think the current tool provides "hooks" as you've described and I think the closest thing would be these "events" although I don't think they fit your use-case.

It could be nice to have a feature to overload docstrings based on decorator docstrings:
I would imagine you could do that by adding a is_decorated property on here.

Or maybe even at this level with something like: astroid.nodes.decorators

@jacobtomlinson
Copy link
Author

jacobtomlinson commented Jul 2, 2024

I think events is what I'm looking for, but maybe the autoapi-skip-member isn't quite the right place to hook this? That one appears to be the only documented event, are there other events?

If the obj argument is mutable then I might be able do to what I'm trying to do. But I can't see an obvious way to find out if a method's class has a specific decorator.

def remove_async_property(app, what, name, obj, skip, options):
    if what == "method":
        # Need to figure out how to check if class that owns the method is decorated with my decorator
        if ...:
            if not name.startswith("_") and not name.startswith("async_"):
                if "async" in obj.properties:
                    obj.properties.remove("async")

def setup(sphinx):
    sphinx.connect("autoapi-skip-member", remove_async_property)

@sachahu1
Copy link

sachahu1 commented Jul 2, 2024

The docs don't seem to point to any other events. From the code point of view, I think this is the only one in here.
I think this is how the autoapi-skip-member function is called in the code, this should help:

def _ask_ignore(self, skip: bool) -> bool:
ask_result = self.app.emit_firstresult(
"autoapi-skip-member", self.type, self.id, self, skip, self.options
)
return ask_result if ask_result is not None else skip

From that it seems that you can "easily" access the docstring on obj and modify it as needed.

Main problem I see is that I don't think there's an easy way to get the decorator from within a PythonObject as this is an already parsed "object". Most examples of retrieving the decorators are done at a higher level while parsing the code. I could be wrong but I think you won't be able to get that out of the box without implementing a feature into the parse_functiondef and passing some extra information into the PythonObject.

As a hacky workaround, you could add something to you docstring (a decorated tag of sort) then use that to overload and modify your docstring within your remove_async_property function.

Hope I could help!

@jacobtomlinson
Copy link
Author

Ok I got things working using the autoapi-skip-member event with a bit of a hack. In my library all the async code is in one submodule, and the sync wrapped code is in another. So I was able to inspect the name and bases attributes to infer whether the class has been wrapped with my decorator and remove the async property.

It would be awesome if the PythonObject kept track of decorators, then my code would be able to know for sure that it was decorated, but making an educated guess seems to be good enough for now.

@AWhetter
Copy link
Collaborator

A similar request was made in #401. It seems like a plugin interface would be useful for users in more than that one case.
Hopefully we'll get this for free if we switch to Griffe (#444).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants