-
Notifications
You must be signed in to change notification settings - Fork 30
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
docs: rewrite Vector's documentation #545
Conversation
Specifically, the API section of the old documentation shows all of these classes: But users of the library don't even need to know that these classes exist. Worse, this could give the impression that users are supposed to access them directly, but they're implementation details. The public part is that Vector objects, arrays, and SymPy expressions have a suite of physics-oriented methods for users to use—nothing more. |
…scikit-hep/vector into jpivarski/rewrite-documentation
…scikit-hep/vector into jpivarski/rewrite-documentation
…scikit-hep/vector into jpivarski/rewrite-documentation
…scikit-hep/vector into jpivarski/rewrite-documentation
…scikit-hep/vector into jpivarski/rewrite-documentation
Since there are no objections, I'll merge it now. Have a great holiday! |
I'll actually give this a read over in the next week, but I think merging now is great as it is better to have improved docs in the wild and incrementally update then stall for "better". Thanks a ton for this work, @jpivarski! Happy holidays! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the amazing work on this, @jpivarski! The documentation looks fantastic now!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we still keep this file and maybe move it outside to the root directory? Helps in keeping track of changes. I will open a new issue for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we can still keep the changelog. I just thought it was redundant with the GitHub Releases page—I don't see why we need both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yes, that makes sense too. My usual workflow used to be -
- Add a CHANGELOG line in every PR.
- Copy the CHANGELOG as release notes when making a release.
But yes, given that there are not a lot of PRs that go in vector's releases, one can just look at GitHub and write the release notes.
What I wrote before updating Vector's documentation
The existing documentation is not physicist-friendly: it starts by talking about backends and instead of giving a list of physics functions, it lists all of the classes that are needed to make it work, including hidden (underscored) modules and protocols that only exist to make the backends generic and possible to type-annotate.
Instead, the documentation should start with a diagram explaining the coordinate systems, examples of how to use it in each of the backends, and then a list of physics functions.
I'm on the fence about including the changelog, too, since that's essentially the same information as the GitHub Releases page, and those who would be looking for that information probably know to look on GitHub first.
Also, the "Structure of Vector" has been an empty page since forever. Although I would have thought that stub pages are neutral, neither good (because they only say that there's more to be said) nor bad (because they don't take away from the other sections, which do say useful things), but we found with the Awkward documentation that stub pages are a net negative. On multiple platforms (including Hacker News), I saw comments in which people inferred from a stub page in the documentation that the library itself was unfinished. That's definitely the wrong impression to give.
What I'm writing now
It's done. I skimped a little on the Numba documentation, but that will probably want an update when #43 is done, anyway.
For reviewers: you can look at the new documentation either by checking out this git-branch and running
and finding it under
docs/_build/html
or by downloading this prebuilt directory and unzipping it (for your convenience). Either way, navigate to that directory and runto view them (to ensure that the CSS loads).
To compare with the old documentation, see https://vector.readthedocs.io/en/latest/index.html and look at them side-by-side. The new documentation has all of the same content, but it's more Apple-style (land on a friendly, uncluttered page with the details behind the links). The tutorials are really tutorial-like—they don't try to tell you everything, since that's what the API reference is for. The API reference no longer shows all of the classes needed to make the backends mypy-typeable—they only show the properties and methods that users need to know about.
I also simplified the README (new version here because I think it was taking up the slack as substitute documentation.
By the way, if I sound hard on the old documentation, it's because I wrote most of it and I didn't intend for it to last this long. @Saransh-cpp added content on the SymPy backend and subclassing Awkward-Vector, and all of that is still present in the new documentation (though I added text to the SymPy introduction).
I think this is ready to merge and get new documentation. I've pinged all of you to give you a chance to look it over and raise any objections. Next week is Christmas break, so it's now or two weeks—if I don't hear negative responses by 5pm tomorrow (Friday), I'll merge it. Anything else and I'll hold off on it and we'll talk in more detail in January.
Cheers!