mkautodoc icon indicating copy to clipboard operation
mkautodoc copied to clipboard

Include type annotations in function signature

Open AnjoMan opened this issue 5 years ago • 26 comments

This MR does two things

  • converts tests to take in readible HTML strings, and compares by parsing and asserting the same elements exist with the same attributes and text content
  • adds type annotations to the rendering of classes and function signatures

I've set up the classes so that users can target only type-annotated signatures -- e.g. adding the following custom css:

.autodoc-param-definition {
  display: flex;
}
.autodoc-signature__annotated .autodoc-param-definition {
  display: block;
  margin-left: 20px;
}

... result in each parameter being put on its own line, but only for signatures with annotations; other signatures could still be rendered as a single line

image

I kept the test refactoring in a separate commit, so you can run the tests against https://github.com/tomchristie/mkautodoc/pull/13/commits/410c7928e6b2e5c7a598e4de239372235d8ebab1 and see that they pass before i do any code changes.

AnjoMan avatar Feb 07 '20 14:02 AnjoMan

@tomchristie are you still working on this repo? or is there someone else who is maintaining it?

AnjoMan avatar Feb 18 '20 22:02 AnjoMan

Thanks for digging into this!

There's a bit too much in the PR to make it easily reviewable. Would it be possible to strip it down to just tackling the "include type annotations" part?

I'm also interested in how the type annotations look in the docs - a screenshot before/after example here would be fantastic. We might(?) want to have them reduced into being click/hover context popovers, so that we don't swamp the docs when there are complex annotations.

tomchristie avatar Feb 19 '20 11:02 tomchristie

I can split the PR up a bit to make it easier. I'll probably pull out the test refactoring part and send you that first, because without it I don't think it will be easy to tell how my changes affect the existing behaviour.

in terms of before-after, I could try to put something together to show you how things will look. I've tried to set up the annotations so that, if you didnt annotate your code then you'd see no change, but if you did annotate it then the docs would look similar to what the function definition would probably look like in the source of it were autoformatted (e.g. using black).

thanks for your comments!

AnjoMan avatar Feb 19 '20 12:02 AnjoMan

@tomchristie FYI, i split out the test refactoring work here: https://github.com/tomchristie/mkautodoc/pull/15

AnjoMan avatar Feb 19 '20 14:02 AnjoMan

Ok, so i've made up some examples of how i think things should shake out. The main things I'm after here are:

  • short function signatures should continue to look nice and compact
  • long function signatures (including one that are long because of annotations) should continue to wrap like they do already
  • if a user wants, they can add styling to make long signatures (including ones with annotations) wrap nicely

This gets us pretty close to rendering signatures exactly as they are written in the authors code -- if they don't annotate their code, there wouldn't be any impact on them, but if they do annotate their code then it should just show up in the docs as they wrote it.

A few examples are described below; i've included a screenshot of the function definition along with what mkautodoc produces if you don't add any custom styling; i've also attached a screenshot of how it could look if the user decides to add styling.

Function with short signature

Here we have a short function signature with annotations. we want it to stay compact and easy to read.

  • function definition image
  • with no custom css image
  • adding custom css image

Function With a Long Signature

Here we have a long signature with no annotations. This is how long functions are already rendered by mkautodoc -- the only thing i've changed is that I've added support for using custom css to improve the readibility by making the signature multi-line.

  • function definition: image
  • with no custom css image
  • adding custom css image

Signature With Lots of Type Annotations

Her we have a signature that is long because of the complicated type annotations. As you can see, mkautodocs renders it exactly like any other long signature definition -- the rendered text flows onto multiple lines. just like any other long annotation, we can write custom css to improve the readibility

  • function definition image
  • with no custom css image
  • adding custom css image

Custom CSS

Here is the custom css i'm using to get my definitions nicely styled

/* indent params in a long signature and put them on their own lines */
.autodoc-signature__long .autodoc-param-definition {
  display: block;
  margin-left: 20px;
}

/* indent the docstring below the signature */
.autodoc > div:not(:first-child) { margin-left: 10px;}



/* style the function signatures */
.autodoc-signature code { color: #404040;}
.autodoc-punctuation {color: grey;}

/* style the definitions such as class */
.autodoc-signature > em:first-child {font-weight: bold; font-style: initial}


/* style type annotations */
.autodoc-type-annotation {
  font-weight: 500;
  color: #9c6a0e;
}

/* style defaults */
.autodoc-param-default {
  color: gray;
}

AnjoMan avatar Feb 19 '20 16:02 AnjoMan

Okay, this looks fantastic!

tomchristie avatar Feb 20 '20 10:02 tomchristie

I reckon this PR should also update the custom CSS in the README, right?... https://github.com/tomchristie/mkautodoc#4-optionally-add-styling-for-the-api-docs

tomchristie avatar Feb 20 '20 10:02 tomchristie

Ok, i've updated the README.md example with the css for long signatures. if someone drops that into their project, heres what they'll see:

image

AnjoMan avatar Feb 20 '20 14:02 AnjoMan

Wonderful. This PR incorperates #15 too, right? I reckon let's just drop the \ in the test multi-line strings, and then get these in.

(Maybe simplest just to merge this one and close the other I guess? I'd be okay with that now that have had the chance to review #15 in isolation.)

tomchristie avatar Feb 24 '20 09:02 tomchristie

@tomchristie ok cool! i've pushed up a change to remove the leading slashes here https://github.com/tomchristie/mkautodoc/pull/13/commits/5536b1e312fe761942d5cc1ea82f420402575b2c. I'll close #15 as well

AnjoMan avatar Feb 24 '20 14:02 AnjoMan

@tomchristie just want to bump this -- i think I've prepped everything to get this done, or is there anything else you think we should get fixed up?

AnjoMan avatar Feb 28 '20 17:02 AnjoMan

Righty. Installed this with httpx, to see how it's looking there...

Eg. rendering this page https://www.python-httpx.org/api/ ...

Screenshot 2020-03-02 at 10 20 04

There's a bunch of awkward edges in how those docs are rendering, so we need to think about how best to move foward.

Some things we might want to consider?...

  • It might make sense to have this as configurable, perhaps even defaulting to off at the moment?
  • We probably want ForwardRef('MyClass') to just document as MyClass.
  • The ~AnyStr style is surprising.
  • Internal import paths may not reflect external usage, eg. httpx._models.Response isn't the user-facing import style.

tomchristie avatar Mar 02 '20 10:03 tomchristie

ok, this is good material. I was just relying on the built-in annotation formatter but it seems like that won't generalize well.

I can also look into what would be a good way of making this a configurable input -- I'm thinking of a flag in mkdocs.yml, but I could also see the case for a colon-type declarative in the docs, so that it would be possible to pick which functions get annotated

On Mon, Mar 2, 2020, 5:28 AM Tom Christie [email protected] wrote:

Righty. Installed this with httpx, to see how it's looking there...

Eg. rendering this page https://www.python-httpx.org/api/ ...

[image: Screenshot 2020-03-02 at 10 20 04] https://user-images.githubusercontent.com/647359/75667558-c84f8000-5c6f-11ea-9676-650b671b9737.png

There's a bunch of awkward edges in how those docs are rendering, so we need to think about how best to move foward.

Some things we might want to consider?...

  • It might make sense to have this as configurable, perhaps even defaulting to off at the moment?
  • We probably want ForwardRef('MyClass') to just document as MyClass.
  • The ~AnyStr style is surprising.
  • Internal import paths may not reflect external usage, eg. httpx._models.Response isn't the user-facing import style.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/tomchristie/mkautodoc/pull/13?email_source=notifications&email_token=AASNOGFHMA2QV2QKPE7R3NLRFOC5LA5CNFSM4KRP2472YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOENOYSUA#issuecomment-593332560, or unsubscribe https://github.com/notifications/unsubscribe-auth/AASNOGBIPN7SWCNVYBKEMT3RFOC5LANCNFSM4KRP247Q .

AnjoMan avatar Mar 02 '20 12:03 AnjoMan

I'm thinking of a flag in mkdocs.yml

I'd run with that as the first pass, yup.

tomchristie avatar Mar 02 '20 12:03 tomchristie

For me, the types are a little hard to discern in the above. A monospace font for the types might make it a bit more legible?

dhirschfeld avatar Mar 04 '20 23:03 dhirschfeld

Ok, so, i've made some more progress on this

  • [x] added a flag include_type_annotations so we can turn this on/off. default is off for now
  • [x] better formatting of annotations -- i just pulled in the implementation from sphinx, since it seems fairly exhaustive. i opted to copy the source over because sphinx is a 2.7mb dependency with a whole bunch of related dependencies that i don't think we need

image

On the point about user-facing imports, I'm not entirely sure what we could do. i think it might be possible to look at the top-level package of the function and see if any types are importable from that package at the top-level, (e.g. sys.modules[Response.__module__].__package__' -> httpxand then try to doimport Response from httpx` using importlib. but this isn't exhaustive, since a submodule of a package could also be the user-facing import

AnjoMan avatar Mar 05 '20 05:03 AnjoMan

@tomchristie what do you think about pulling the user-facing-imports part of this out into a separate issue? i think its worth doing, but it seems like it would take a bunch of research and work to figure out, and what we have already serves the use-case of documenting functions that use standard types.

AnjoMan avatar Mar 09 '20 13:03 AnjoMan

On the point about user-facing imports, I'm not entirely sure what we could do.

There's not going to be any general purpose way to figuring out what the import string ought to be. One answer could be to just always drop the leadning modules, and only document the class name? (eg. CookieJar and Response in the example above.)

tomchristie avatar Mar 09 '20 15:03 tomchristie

@tomchristie Does this look ok to merge?

ofek avatar Mar 16 '20 14:03 ofek

@tomchristie just bumping this. i think i've covered all the issues, we have now:

  • a cleaner type-hint rendering function (no more ~Any)
  • package structure is cleaned out of annotations (package._secret_module.ClassName shows as ClassName)
  • a config option for whether to render type-hints at all, with default as false --- nothing should change for existing users unless they add the option

AnjoMan avatar Mar 28 '20 14:03 AnjoMan

Really looking forward to this PR, especially the type renderings!

We'll be using it almost immediately in adaptnlp as everything is type hinted and we rely on mkautodocs for class api documentation

aychang95 avatar Apr 02 '20 04:04 aychang95

Would be nice to have this.

alanjds avatar Jun 01 '20 16:06 alanjds

I'd love to start making use of this functionality. Is there any objection to merging this as-is and then iterating on master (if required)?

dhirschfeld avatar Jul 15 '20 07:07 dhirschfeld

+1 for this functionality. :)

patrick-kidger avatar Nov 04 '21 22:11 patrick-kidger

Hi @patrick-kidger :wave: At this point, I think this repo is ~unmaintained (no disrespect intended to the authors) and that the best option currently for autodoc like features for mkdocs is https://github.com/mkdocstrings/mkdocstrings

dhirschfeld avatar Nov 05 '21 10:11 dhirschfeld

Thanks :) Yeah, that's my impression. (FWIW I've tried mkdocstrings but haven't managed to get it working.) Now I'm considering rolling my own... :D

patrick-kidger avatar Nov 05 '21 18:11 patrick-kidger