Skip to content

One hookspec to many hookimpl (implementations that don't match spec function names) #218

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

Closed
ghost opened this issue May 29, 2019 · 7 comments

Comments

@ghost
Copy link

ghost commented May 29, 2019

This is a proposal for an enhancement. Before I make my request, thank you for all the work associated with this project.

My use case involves the need to support any number of hook implementations, which are essentially operations for an application to perform. For example:

  • someapp_operation_do_something
  • someapp_operation_do_something_else
  • someapp_operation_do_something_different
    ... etc

Maintaining flexibility here is important for our use case. Having to go back and create a spec every time someone wants to implement a new operation would be cumbersome. It would be great if I could define a hookspec like:

    @hookspec
    def someapp_operation(self, context):
        pass

And then the implementations became something like this:

    @hookimpl(spec="someapp_operation")
    def someapp_operation_with_a_different_name(self, context):
        do_some_stuff()

If I happen to call an operation that doesn't exist (wasn't actually implemented), then I can deal with the AttributeError. That being said, it gets us away from the need to maintain N specs in our application.

The other way that I was thinking that this could be handled is just having each operation implement someapp_operation, but that would probably be something like:

    @hookimpl
    def someapp_operation(self, operation_name, context):
        if operation_name == "some_operation_name":
            do_something()

This just feels really ugly to me and prone to error.

I did some initial research into _HookCaller and set_specification(), but I won't get ahead of myself until it is clear that this might be worth investigating further and that there isn't already a pattern for handling this.

Thanks again!

@RonnyPfannschmidt
Copy link
Member

The use case is absolutely unclear to me,
with the provided information the only answer I personally have is a strong -1

I'm happy to change my mind if adequate extra info is presented

@ghost
Copy link
Author

ghost commented May 29, 2019

The general use case is being able to define a hookspec once and then enforce that hookspec for any number of hookimpls. My use case happens to require a large number of hookimpls (say hundreds) that can be defined by those writing plugins. Assuming that I actually knew all of these use cases upfront, I would have to do something like the following to support them all:

    @hookspec
    def someapp_some_operation1(self, context):
        """Something about operation1"""

    @hookspec
    def someapp_some_operation2(self, context):
        """Something about operation2"""

    @hookspec
    def someapp_some_operation3(self, context):
    """Something about operation3"""

   ... for many, many operations

This would be a lot of repetition and requires code changes every time a new operation needs to be handled. I could decide to skip the hookspec altogether, but now I don't get any validation at all.

The use case to support a large number of hookimpls is that the host application could be sent any number of operations at runtime. Assuming specs live in the host application, it would be nice not to have to update the host application because a plugin maintainer wants to support a new operation.

Please let me know if there are specific parts of the use case that don't make sense. I might be missing an obvious way to handle this.

@RonnyPfannschmidt
Copy link
Member

based on the abstract use-case its rather evident, that this is no the use-case that pluggy is trying to solve

whats the concrete use-case?

@ghost
Copy link
Author

ghost commented May 29, 2019

The concrete use case is an IoT device management agent. A device can be sent a message to execute "some_operation" and it would have attempted to execute PluginManager.hook.some_operation().

There are simply many different types of devices, and subsequently many different maintenance operations to be performed on these devices. If we were to use pluggy, I wanted to avoid becoming the gatekeeper for creating specs for every maintenance operation that someone might want to implement.

There are certainly other ways to solve this, but I figured I would start here to make sure that this wasn't something that was being considered. I'd always prefer to contribute to an existing project over starting from scratch, but I understand this isn't something that this project is trying to solve.

Thanks for the consideration!

@RonnyPfannschmidt
Copy link
Member

@saf42 plugins for devices can add device specific hookspecs as well - its possible to layer it reasonably having clear specs for new device types

its just not intended to do this dynamically at the hook declaration layer

@tlambert03
Copy link
Contributor

tlambert03 commented Jan 26, 2020

I was actually just about to submit a PR for this same feature, but will start here with my reasoning/use-case first:

some of the potential hookspecs in our application (napari) might well declare signatures and APIs that other libraries are actually already implementing, without having been explicitly trying to write a hookimpl for our program. That is to say, some of these hooks are rather general (e.g. accept a filepath, return a numpy array), and many libraries are probably already implementing functions that satisfy our hookspecs. Currently, in order to act as a plugin to our program, other libraries need to create new modules/functions as aliases to their existing functions, where the new function names follow our naming conventions. As a toy example:

# other_library/io.py: 
def imread(path):
    array = do_something_with(path)
    return arrray

# other_library/napari_plugin.py:
from .io import imread

@napari.hookimpl
def napari_read_file(path):
    return imread(path)

However, if they could decorate their existing functions with a hookimpl that accepts a hookspec name as an argument, they could mark their existing functions as being compatible with our API:

# other_library/io.py: 
@napari.hookimpl(spec="napari_read_file")
def imread(path):
    array = do_something_with(path)
    return arrray

# other_library/napari_plugin.py no longer needed

In other words, it lets commonly-implemented APIs be flagged for external use, without additional code/aliases/boilerplate. Thoughts?

@bluetech
Copy link
Member

Fixed by #251 - use @hookimpl(specname="napari_read_file").

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

No branches or pull requests

3 participants