Remove manual kwargs-only check?

A little trivial issue.

The hookcaller __call__ (e.g. pm.hook.myhook(...)) requires keyword-only arguments. This is currently checked manually:

pluggy/src/pluggy/_hooks.py

Lines 384 to 386 in 172ca9f

    
           def __call__(self, *args: object, **kwargs: object) -> Any: 
        
               if args: 
        
                   raise TypeError("hook calling supports only keyword arguments")

and provides a nice exception message (though it could be nicer...).

An alternative is to make the function itself kwargs-only:

    def __call__(self, **kwargs: object) -> Any:

then Python itself checks it. This provides a worse exception message: _HookCaller.__call__() takes 1 positional argument but 2 were given (it's still possible to find the line which has the problem from the stacktrace).

On the other hand, the Python kwargs-only approach has the better typing (i.e. mypy will be able to catch bad usages), and removes the slight overhead of the check (likely not significant though).

I'm inclined to make the change but wonder if others agree.

I'd like to rearrange the hook caller class hierarchy a bit to ensure that the hookcaller name turns to a protocol

Then we can leave the user friendly error for now and split details like historic calls, hook wrapping and more

I'm 👍 on the change.

I'd like to rearrange the hook caller class hierarchy a bit to ensure that the hookcaller name turns to a protocol

I think this might be a good idea, for the subset caller at least.

Then we can leave the user friendly error for now and split details like historic calls, hook wrapping and more

If you can flash it out in more details, I'm interested to hear; not sure exactly what your envisioning with the splitting here (class hierarchy or composition/wrappers? Breaking or keeping backward compat? And how can hook wrapping be split?)

I'll make a distinct issue for that

It plays into a combination of wanting async hooks, hooks with more native multi results as well as hookwappers that can filter

I'll have to bounce that off @simonw as well

But as for the kwargs as python native, we can go with that as well,

That kwargs change looks sensible to me.

I'm intrigued to hear more about the async hooks proposal! I'm using a bit of a hacky workaround for that at the moment:

@hookimpl
def extra_template_vars(datasette, database):
    async def hidden_table_names():
        if database:
            db = datasette.databases[database]
            return {"hidden_table_names": await db.hidden_table_names()}
        else:
            return {}
    return hidden_table_names

Then when I call the hook:

        for extra_vars in pm.hook.extra_template_vars(
            template=template.name,
            database=context.get("database"),
            table=context.get("table"),
            columns=context.get("columns"),
            view_name=view_name,
            request=request,
            datasette=self,
        ):
            extra_vars = await await_me_maybe(extra_vars)
            assert isinstance(extra_vars, dict), "extra_vars is of type {}".format(
                type(extra_vars)
            )
            extra_template_vars.update(extra_vars)

See https://simonwillison.net/2020/Sep/2/await-me-maybe/

	def __call__(self, args: object, *kwargs: object) -> Any:
	if args:
	raise TypeError("hook calling supports only keyword arguments")