pynacl
pynacl copied to clipboard
pyca
use docstrings or separate .rst files for docs?
In #73 @lvh noticed that #77 removed a bunch of docstrings, in favor of separate .rst files. Are we cool with this? I agree with @lvh that docstrings are awfully handy. (I hadn't realized that removing the "autodoc" code was related to removing docstrings).
Personally, I'm -0 on removing docstrings. I like having API summaries in the docstrings, and then longer-form prose/tutorial/examples in the standalone files.
I'll volunteer to write docstrings if someone tells me what format I should use. Maybe they'd be easier to write/maintain if they don't need to be machine-readable?
Here is my perspective:
Docstrings should not be confused with documentation for users. This is for a few reasons:
- Documentation almost always need to have "cross functional synergy", where multiple component's combined behavior is documented as one system, for the benefit of the user
- Putting documentation into a separate file forces a separate state of mind when authoring it
- Docstrings tend to privilege the program author, and their concerns, above the user. This is for a pretty simple reason: you're looking at the source when you write the docstring.
- The things you need when writing a piece of code are not the same as the things you need when using that code.
- Empirically, projects which write separate prose documentation universally (in my experience) have better docs than those using docstrings.
Please feel free to write docstrings, but I think they should be a utility for the program author, and not to be confused with actual documentation, they are a summary format for code comments.
For the record, I'm not opposed to having doc-strings or anything. I just don't want to try and maintain the old doc-strings which were designed to be end user documentation via autodoc.
Sounds good. How about we release 0.3.0 without the docstrings, but then I'll go through and make up API summaries for docstrings to be added for the subsequent release?
I don't think anyone disagrees on the prose/docstring dichtomy. The docstrings that were there definitely had some bits that should have been documented elsewhere.
However, stuff that was in there such as parameter definitions and return types are useful to someone using the code or developing it. Right now, if I'm writing some code that calls some pynacl functions, and I'm in my editor, I see a useful docstring. If I'm in a REPL, it's really easy to show a useful docstring. Now, it doesn't do anything like that, and I'm convinced that's a regression.
Yeah, I'd agree that's a regression. I guess the question is whether it's one we should fix for 0.3.0, or whether it'd be ok to ship an 0.3.0 without the docstrings and then add them back into a later release.
As much as I'd like to make a new release available soon, I also like docstrings when writing code, so I'd be ok with making this a blocker for 0.3.0.
The problem is while parameter definitions and return types are somewhat useful to have in a docstring, that information has to be in the .rst files given the autodocs problem.
Maintaining two copies of that information is just asking for them to get out of sync.
I also think everyone agrees that writing good documentation is hard work, and good docstrings as well as prose documentation is only going to exacerbate that. (I would argue keeping docstrings up to date is easier than just keeping the prose docs up to date, since you have to deal with the docstrings when you're hacking on the code.)
This is my main motivation for using autodoc for PyOpenSSL:
- Those docs are de facto API docs anyway, not prose, leading to literal duplication, not just partial duplication.
- There's not enough manpower on the project to review the patches that exist let alone write and maintain both a ton of prose documentation and the docstrings.
- Ideally, if anyone were to at some point feel the need to write a bunch of prose documentation, they wouldn't be helped, not hindered, by the docstrings being good.
I think all of this perfectly reasonable as long as someone is willing to step up to write and maintain the docs :) Even if it is okay to release a version that has a bunch of docstrings removed from it, would that only be a temporary thing? Would someone eventually have to step up and write a bunch of docstrings? Seems to me that either you just give up on doing that, or that maintenance burden isn't going away, merely being deferred.
I personally feel that a bunch of docstrings that were there and are now just nonexistent is a pretty bad regression. Not omg-pynacl-is-unusuable bad, but definitely "man why did anyone do this" bad. But hey, like I said: not a maintainer, that's just, like, my opinon, man :wink:
Maybe as a side note, it sounds like we're talking about two different causes here:
- Have prose, because prose leads to better docs.
- Don't have autodoc, because autodoc imports code (which doesn't work for a variety of reasons).
Those are pretty different motivations.
Yeah, I'm ok with not machine-parsing the docstrings (and I don't know enough to have an opinion on autodoc, except for thinking that doing fewer imports is a good thing).
@lvh your opinion is important to me :). I'm going to add this issue (e.g. adding docstrings to replace the ones that got removed during the autodoc-ectomy) to the 0.3.0 list.
Crazy thought here. Is it possible to do the reverse of autodocs? I.e building the docstrings from the .rst files dynamically each time tox -e docs is ran?
Hm. I suppose we could, but that would have a similar and perhaps worse problem of not only having to find where the docstrings are, but also
Perhaps there's some other tooling we could re-use. For example, davidhalter/jedi is in the business of understanding most Python code without importing it. While it doesn't cover all the cases that regular Python code would when importing (because you can do all sorts of ridiculous things including exec to get a class bound to a name...), perhaps it could access docstrings. I certainly know it parses them, and that @davidhalter has historically been very helpful when I attempted to use/improve/fix jedi :)
@davidhalter: tl;dr can we use jedi to get docstrings for classes, methods, functions, modules without importing them so we don't have to use autodoc?
Also all of this would be nice (because yay let's make autodocs a bit better), but, just to be clear (since it's hidden over a bunch of tickets): have we tried to work around this issue by mocking out the relevant modules? I remember having to do this when building docs using autodocs on very limited build hosts, and it sounds awfully similar to what is going on in #52.
Here's an article describing it for numpy: http://blog.rtwilson.com/how-to-make-your-sphinx-documentation-compile-with-readthedocs-when-youre-using-numpy-and-scipy/
@davidhalter: tl;dr can we use jedi to get docstrings for classes, methods, functions, modules without importing them so we don't have to use autodoc?
Yes, that's possible. jedi.defined_names should be able to help you. If you need to list names of a class, we might need to add something to Jedi's API, but that's possible (you could even hack around it, now).
So I guess two questions that come to mind are:
- Why's sodium_init have to be called in the module body? (Okay maybe because you want to make sure it's been called at least once and you don't want to check that every method call...)
- Why can't lib.sodium_init() (granted, the mock, when building docs) return 0?
We want to ensure that sodium_init() has been called, but more importantly we also want to ensure that it's not called at the same time in two different threads. It is safe to call it more than once, but the function itself is not thread safe. By calling it in the module body we ensure that it is called without incurring the need to check that it has been called on every function call. We also get to rely on the import lock which Python has to ensure that a module import only happens in one thread instead of having to handle locking ourselves.
As to your second question, I never tried to do that. @Ayrx said that it was causing problems and I just trusted that.
Okay, that's a good reason for calling it in the module body.
So, then we're left with: when building docs, lib.sodium_init() should be 0, unconditionally, without really doing anything. Can we do that? (cc @Ayrx)
That should be possible I suppose... You can see the code that does the mocking in #85. If someone wants to play around with it and make it work, feel free.
So, I thought about this some more: assuming you don't have autodoc, can't we just reinstate docstrings? I mean, it's autodoc doing the imports.
@lvh Yes. I think the main concern with that was the docstrings and .rst files getting out of sync. If the primary maintainer (which is basically @warner at this point I think?) and the reviewers are vigilant this should be ok I suppose.
Probably :)
So I guess the remaining issue is that while I happily accept that prose docs are great and can't be replaced with some autodoc, the docs that #76 and #77 introduce aren't those prose docs: they are literally just the docstrings, except no longer in the shape of docstrings. If someone wants to write some good prose documentation, super; but if I understand correctly that means there's literally no benefit to doing it this way besides "it works around the mocking problem"?
More or less yes, to my recollection. I think I did tidy up some stuff and document some stuff that weren't documented before but it's to avoid the mocking problem. Someone may want to fix that and switch back to autodocs or add docstrings and keep them in sync, both seems fine to me.
Unfortunately I don't have the time to do this right now, so someone else will need to undertake this.
I'm volunteering: I just wanna make sure I'll be doing the right thing :-)
So, I would suggest going back to autodoc in this case, since these specific cases are really docstrings (they've just been transplanted). We should have prose docs at some point too, but that's an entirely different matter :)
I'll leave the final call up to @warner ; I'm volunteering no matter what that call is.
Boy, did I get tagged with the maintainer stick again? :)
Lets copy the non-prose text back into the docstrings, so they'll be available from the repl and help() and pydoc. Let's leave the same non-prose text in the .rst files, and plan to replace them with better more-prose less-API-ish less-autodoc-ish text in the future.
I'm neutral on turning autodoc back on (I've never used it myself, I don't really know what it outputs or how easy that is to use).
Looks like PR #99 only restored signing, so we still need to restore the docstrings for hash, secret, and public. I'll make a PR to do that now.
#126 restored the docstrings. Nothing left in this issue needs to be in 0.3.0, so I'm removing the milestone flag.