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

Autodoc #203

Merged
merged 2 commits into from
Jul 31, 2020
Merged

Autodoc #203

merged 2 commits into from
Jul 31, 2020

Conversation

pawamoy
Copy link
Contributor

@pawamoy pawamoy commented May 6, 2020

Issue #173

We use mkdocs to write and build the documentation, with the material theme for mkdocs, as well as the mkdocstrings plugin for auto-documentation of the source code.

I had to remove the doctoc pre-commit hook because I couldn't get its generated tables of contents to be rendered correctly in the documentation (they are a mix of HTML and Markdown). Note that this is not a great loss, as the material theme for mkdocs adds its own table of contents in the right sidebar for every page. We lose the ToC on GitHub only (and it was not really usable either because it's not sticky).

To try it, clone my branch and run poetry run mkdocs serve, then go to localhost:8000.

I could use early feedback on the following TODO points:

TODO:

  • Improve the code documentation: some modules have no docstrings at all, so they appear empty in the docs. The other docstrings could really be detailed a bit more. We can also benefit from the cross-reference ability of mkdocstrings, linking objects between them to improve navigation in the reference.
  • Customize the docs? Version 5 of mkdocs-material makes it easier to customize the docs color and icons. I'd be more inclined to let the authors/maintainers of copier do so 🙂
  • Split the README (aka overview in the docs) into multiple files? Create a template, generate a project, update a project, etc..., then clean it up (remove API section maybe).

Screenshot_2020-05-06_14-49-51

@pawamoy pawamoy marked this pull request as draft May 6, 2020 12:01
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Improve the code documentation: some modules have no docstrings at all, so they appear empty in the docs. The other docstrings could really be detailed a bit more. We can also benefit from the cross-reference ability of mkdocstrings, linking objects between them to improve navigation in the reference.

Well, I guess that is something that wasn't done before just because we had not this stuff in place.

I'd appreciate some example(s) on how that linking works.

Also there's one thing that calls my attention: typing annotations appears nowhere in the docs. But all our code base is typed. Can't that be configured? Typing enables easy cross-linking when using my IDE, it should be able to do the same for docs somehow, theoretically.

Customize the docs? Version 5 of mkdocs-material makes it easier to customize the docs color and icons. I'd be more inclined to let the authors/maintainers of copier do so slightly_smiling_face

Don't worry about that for now 😉

@pykong could say something about this if he cares. BTW are you OK @pykong? I'm worried, you're very absent since Covid-19 started... 🙄

Split the README (aka overview in the docs) into multiple files? Create a template, generate a project, update a project, etc..., then clean it up (remove API section maybe).

Yes, now with proper docs, README should be much shorter.

There's one thing missing: please add some Github action workflow that uploads these docs. AFAICS it should be quite easy to do: https://www.mkdocs.org/user-guide/deploying-your-docs/

If there were a way to produce some browsable artifacts for PRs and upload docs just for pushes in the master branch, that'd be just awesome.

BTW, any chances to get docs per version (from now on)? 🤔

Thanks! Your contributions are being so great!

README.md Outdated Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
@pawamoy
Copy link
Contributor Author

pawamoy commented May 7, 2020

I'd appreciate some example(s) on how that linking works.

Sure. Simply put, you can link to any heading anywhere else in the site with the classic Markdown syntax, and since each Python object is rendered with a heading, you can link to them:

  • with a custom link title: [`EnvOps` class][copier.config.objects.EnvOps]
  • or directly: [copier.config.objects.EnvOps][].

This will work both in mkdocs Markdown pages and in Python docstrings (as long as they are valid Markdown).

And "any heading" means you can also link to other headings, for example: [See the Get Started section in Contributing][get-started].

This will continue to work even if headings are moved from one page to another, or if a Python object is moved from one module to another. mkdocstrings will warn if it finds references that cannot be resolved, for example [see this][this-heading-does-not-exist].

typing annotations appears nowhere in the docs

They do not appear in the generated docs signatures, no, and it's not configurable in mkdocstrings (yet). However we're supposed to write the docstrings in the Google format. When doing that, mkdocstrings render the docstring with tables for parameters, return value and exceptions, using type annotations in the signature, something like this: https://pawamoy.github.io/mkdocstrings/reference/handlers/python/#mkdocstrings.handlers.python.PythonRenderer.render

It means that if we follow the Google-style and document each parameter in the docstrings, they will indeed be rendered in the generated docs 🙂

Yes, now with proper docs, README should be much shorter.

OK, I'll try to split the README then.

There's one thing missing: please add some Github action workflow that uploads these docs. AFAICS it should be quite easy to do: https://www.mkdocs.org/user-guide/deploying-your-docs/

Good idea!

If there were a way to produce some browsable artifacts for PRs and upload docs just for pushes in the master branch, that'd be just awesome.

Just to be sure, you want to deploy the docs for each push on master? I'd favor deploying when pushing tags only, and it's related to the next point ->

BTW, any chances to get docs per version (from now on)?

Unfortunately, I didn't find any mature solution allowing to version the docs. About the previous point, if we have only one deployed version at a time, I think it would be best to document the latest release, and not the current commit (master branch). Of course if we manage to deploy multiple versions, having docs for each release and for the master branch is the best.

Now, for the things I tried:

  • mike, but I don't remember what the result looks like. I'll try it again.

Also, I don't quite agree with their idea:

mike is built around the idea that once you've generated your docs for a particular version, you should never need to touch that version again.

When vulnerabilities are found, you absolutely need to modify the docs for the impacted versions, to warn users. Same when a version is deprecated and not maintained, you really should display a banner discouraging people to use this version, and to upgrade to a newer one. And again, in the development version (master branch), you should warn users that this version is the development one, and this warning needs to go away once it's released. Finally, mistakes can be made while writing docs, and I think developers should always be able to correct them, even if this version of the docs has already been deployed. Wrong docs are worse than no docs. Anyway, that's just my opinion 😄

  • mkdocs-versioning, but the project seems to be on hold, and I don't quite like how it works: it writes each version in a subfolder (:+1:) so they can be reached using sub-URLs (:+1:), but to switch from one version to another you have to click on a Versions link in the sidebar, then click again on the desired version (:-1:). You cannot switch version while staying on the same page or anchor (:-1:).

@yajo
Copy link
Member

yajo commented May 8, 2020

When vulnerabilities are found, you absolutely need to modify the docs for the impacted versions, to warn users. Same when a version is deprecated and not maintained, you really should display a banner discouraging people to use this version, and to upgrade to a newer one. And again, in the development version (master branch), you should warn users that this version is the development one, and this warning needs to go away once it's released.

I think your concerns about mike are more theoretical that practical. 🤔

See, if we rebuild the gh site on each commit to master, AFAIK we have to rebuild the whole site. That includes static pages for previous versions.

Also, those problems you mention can be easily fixed by maintaining pages like "supported versions" or "vulnerabilities" statically in the latest version only (if that's possible).

If we build on each commit, and the build includes all tags, then if that commit has a tag and we are able to default to that latest tag, it should be just OK.

Also I can see that mkdocs itself is documented in readthedocs and, if you browse https://readthedocs.org/projects/mkdocs/ in several versions, you'll see it has a floating version switcher. Maybe that feature is added automatically by readthedocs? If so, we could use it for docs and forget about the problem.

Otherwise we can leave this improvement for the future when mkdocs/mkdocs#193 is fixed upstream.

They do not appear in the generated docs signatures, no, and it's not configurable in mkdocstrings (yet).

I opened mkdocs/mkdocs#2097 upstream, for now using the google style should be enough, although double-writing types is not fun. Maybe we can just use Google syle without types, and just benefit from that upstream improvement if/when implemented. Anyways, your IDE should give you enough type hints when needed.

@pawamoy
Copy link
Contributor Author

pawamoy commented May 8, 2020

Oh, @yajo the auto-documentation is solely the responsibility of mkdocstrings, not mkdocs. You should have opened an issue there 🙂

@yajo
Copy link
Member

yajo commented May 8, 2020

Oops thanks! I'm new to all that stuff...

@pawamoy
Copy link
Contributor Author

pawamoy commented May 8, 2020

I think your concerns about mike are more theoretical that practical.

They are indeed 😄 I need to play a bit with it again.

if we rebuild the gh site on each commit to master, AFAIK we have to rebuild the whole site. That includes static pages for previous versions.

Not sure this is possible with mike:

mike is designed to produce one version of your docs at a time

...and each command it provides expect one and only one specific version as argument.

Also, those problems you mention can be easily fixed by maintaining pages like "supported versions" or "vulnerabilities" statically in the latest version only (if that's possible).

Not a lot of people would go to the "vulnerabilities" page to check it, even less think that such a page exists.


Anyway, I forgot about ReadTheDocs, but yes, this could work just fine, and we would have versions support!

@yajo
Copy link
Member

yajo commented May 10, 2020

each command it provides expect one and only one specific version as argument.

Can we do something like this?

for v in git("tag"):
    mike(v)

Anyway, I forgot about ReadTheDocs, but yes, this could work just fine, and we would have versions support!

What would you prefer then? You are more expert here in documenting. 😁

@yajo
Copy link
Member

yajo commented Jun 17, 2020

Hey friend, I've been gradudated by @pykong to owner here (thanks pal! ❤️). I have all needed permissions now to link with readthedocs if needed, although I think you're the expert in that sense. Think you could continue here?

@pawamoy
Copy link
Contributor Author

pawamoy commented Jun 17, 2020

Hey friend! I was glad to hear good news from you and @pykong 🙂!

Yes, I'll eventually continue the work on this PR, unfortunately my time is quite limited these days because of work 😕

@yajo
Copy link
Member

yajo commented Jun 18, 2020 via email

@yajo
Copy link
Member

yajo commented Jul 22, 2020

Hi friend, this is a kind reminder that whenever you can continue with this, it would be awesome. 😊 Copier is getting some traction and I see some questions like #231 that could easily fit into some FAQ in the docs, if we had them.

Thanks for your help! ❤️

@pawamoy
Copy link
Contributor Author

pawamoy commented Jul 22, 2020

Hi, thanks for the reminder! I'll dedicate some time this evening!

@pawamoy
Copy link
Contributor Author

pawamoy commented Jul 23, 2020

@yajo, I think you can now already enable the project on ReadTheDocs. Did you do such a thing already? Otherwise I can guide you. You just have to login with GitHub on ReadTheDocs, then "Import" the project. Make sure to check the "Advanced" checkbox during the setup, and select MkDocs instead of Sphinx.

@pawamoy
Copy link
Contributor Author

pawamoy commented Jul 23, 2020

Alright, I split the README and created three pages: creating a template, generating a project, and updating a project. The rest stayed in the README, which is also the overview in the docs. I got rid of the auto-generated table of contents which were not rendered correctly in the docs. I also added docstrings and type annotations to the code. It's not perfect but it's a good start!

Waiting for RTD to be linked to the project, and for your review of course 🙂

@yajo yajo self-assigned this Jul 24, 2020
@yajo yajo added enhancement maintenance Boring maintenance routine labels Jul 24, 2020
@yajo yajo added this to the v5.0.0 milestone Jul 24, 2020
@yajo yajo marked this pull request as ready for review July 24, 2020 05:47
Copy link
Member

@yajo yajo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I moved your PR out of draft then.

I have created https://copier.readthedocs.io/ without problem, but I guess I must merge the PR before I build the docs, so I just commented some little details.

Most comments are simply about respecting https://www.python.org/dev/peps/pep-0257/#multi-line-docstrings

Please update to merge. Thanks a big lot! ❤️

@@ -172,12 +222,13 @@ class CopierUpdateSubApp(cli.Application):
generated since the last `copier` execution. To disable that, use `--no-diff`.
"""

only_diff = cli.Flag(
only_diff: cli.Flag = cli.Flag(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type seems to not be detected properly:

2020-07-24_08-13

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, my intuition is that the @CopierApp.subcommand("update") decorator has some responsibility in this 🤔

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not something critical, but maybe we should open an issue upstream... Do you feel like doing that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure, issue opened here: mkdocstrings/pytkdocs#60!

mkdocs.yml Outdated Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
copier/cli.py Outdated Show resolved Hide resolved
@@ -42,21 +66,21 @@ class CopierApp(cli.Application):
CALL_MAIN_IF_NESTED_COMMAND = False
data: AnyByStrDict = {}

answers_file = cli.SwitchAttr(
answers_file: cli.SwitchAttr = cli.SwitchAttr(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised about these changes. I'd say this is a bug on mkdocs because mypy doesn't yell about this. cli.SwitchAttr is a class, so it's pretty obvious that this attribute will have that class as a type.

Any ideas on this subject?

Copy link
Contributor Author

@pawamoy pawamoy Jul 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, mkdocstrings is not as powerful as mypy, and is only using actual type annotations, not deducing them from the value of the attribute. So if you want the type to appear in the docs, you have to annotate it, or add it inside parentheses in the docstring.

Copy link
Contributor Author

@pawamoy pawamoy Jul 28, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually pytkdocs could be improved to use the attribute's type if no annotation is found. I'll open the related feature request 🙂. Issue opened here mkdocstrings/pytkdocs#59

copier/main.py Outdated Show resolved Hide resolved
copier/main.py Outdated Show resolved Hide resolved
copier/main.py Outdated Show resolved Hide resolved
copier/main.py Outdated Show resolved Hide resolved
copier/tools.py Outdated Show resolved Hide resolved
@yajo
Copy link
Member

yajo commented Jul 29, 2020

Could you please rebase and/or squash? It seems like the bots got stuck, I don't know why 🤔

@pawamoy
Copy link
Contributor Author

pawamoy commented Jul 29, 2020

Done!

@yajo yajo merged commit b94b7f4 into copier-org:master Jul 31, 2020
@yajo yajo modified the milestones: v5.0.0, v4.1.0 Aug 10, 2020
@pawamoy pawamoy deleted the autodoc branch April 2, 2024 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement maintenance Boring maintenance routine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants