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

simplify doc generation #38

Open
faassen opened this issue Sep 14, 2016 · 1 comment
Open

simplify doc generation #38

faassen opened this issue Sep 14, 2016 · 1 comment

Comments

@faassen
Copy link
Member

faassen commented Sep 14, 2016

Dectate contains a sphinx extension to help generate dectate documentation. It's unmaintainable grotty sphinx plugin code. It takes care of two things:

  • get the docstring from __init__ of the action.
  • get the argument list correct (the __init__ arguments without self).

We can fix the docstring issue by modifying the __doc__ setting behavior in app.py, for instance on the no-more-directive-directive branch:

    action_factory_doc = action_factory.__doc__
    action_factory_init_doc = None
    if hasattr(action_factory, '__init__'):
        action_factory_init_doc = action_factory.__init__.__doc__
    method.__doc__ = ((action_factory_doc or '') +
                      (action_factory_init_doc or ''))

Unfortunately the argument list is harder to get right. But we recently started to generate functions in Reg with the right signature, and we could do that in Dectate too. The drawback is that this generated function won't be very pretty in the debugger but it's probably not very comprehensible anyway due to the deferred nature of directive execution. If we could generate the correct list of arguments for our wrapper function, we could do away with the sphinx extension entirely. We'd do a bit of function generation when we import Morepath, but that seems tolerable.

@faassen
Copy link
Member Author

faassen commented Sep 15, 2016

Okay, I tried to make code generation work. There are two problems:

  • we now get a TypeError if we call the directive with the wrong arguments immediately. That in itself is good, but the type error uses the generic name method() takes exactly 2 arguments. At this point, the true name of the directive is not yet known, as this is only fixed up during commit. I could use the action class name in the error message, but that doesn't really help the user.
  • the with statement story doesn't work anymore. The whole with statement depends on the fact that you can give partial arguments and then fill them in later. This fails now as it immediately reports you don't have enough arguments.

To work around the with issue we could just drop it and point people to functools.partial, so that:

with MyApp.foo(model=Dummy) as foo:
    @foo(name='a')
    def f():
        pass

    @foo(name='b')
    def g():
        pass

becomes:

import functools
foo = functools.partial(MyApp.foo, model=Dummy)
@foo(name='a')
def f():
    pass
@foo(name='b')
def g():
    pass

though that doesn't use a context manager. It is a bit more verbose, too. We could even introduce a context manager (http://stackoverflow.com/questions/9893904/partial-application-of-functions-inside-a-context-manager-with) but that seems overkill.

Nonetheless, even if do change abbreviated directives, we still have a problem with the type error message. It seems difficult to do something about that, because during function generation time we really don't have access to the name it will eventually get.

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

1 participant