mphowardlab / azplugins

A HOOMD-blue component for soft matter simulations.
BSD 3-Clause "New" or "Revised" License
21 stars 13 forks source link

Sphinx documentation #29

Closed mphoward closed 4 years ago

mphoward commented 4 years ago

Setup framework to build sphinx documentation. Still need to setup readthedocs support and proofread everything, but the docs do compile. Also need to add a docs unit test.

Resolves #1 .

mphoward commented 4 years ago

@astatt Here is a shell of documentation that we can start to work on. Could you take a look and see if you like the overall layout? It should compile with standard sphinx + sphinx_rtd_theme from pip. If so, I can work more on editing and filling in. I've also proposed a section for tutorials and examples, where we should be able to incorporate your jupyter notebooks or writeups.

astatt commented 4 years ago

Good news! It compiles without issues.

I'm happy with the general "Installation/Tutorial/Examples".

The only comment I have about structure is this: Because it is auto-generated, I am not sure if we can do or even should do something about it, but I'm not sure if "Restrains" make sense under "Potentials". Technically, yes, but conceptually they are different enough to feel like they would deserve their own separate section.

Everything else looks good, I'd say merge and then fill in the details and fix typos as we go.

Typos I saw (for future reference to fix): azplugins.bond.fene24 - equation is broken azplugins.special_pair.lj96 - same azplugins.pair.lj96 - equation is missing spaces azplugins.pair.lj124 - same azplugins.pair.spline- equation and math symbols need reformatting azplugins.pair.two_patch_morse - there has to be a better way of writing that equation azplugins.wall.colloid - equation is missing a linebreak azplugins.restrain.plane- second equation is broken azplugins.flow.reverse_perturbation - link to paper is broken azplugins.mpcd.reverse_perturbation -same

mphoward commented 4 years ago

Thanks! I went back and forth on whether restraints should be their own page. (We can choose how we want to group things together, for sure.) If you think they deserve being their own concept, let's separate that part out. I will also fix these small typos for now since that is quick, and we can focus on larger content improvements in later PRs.

One section that I noted is missing is developer topics ("how do I add X?"), so I'll add an empty index page for that. We also need a page for the changelog, which should just import the contents of the file that we already keep, maybe converting it to rst from md.

I would also like to setup a doc unittest and readthedocs hosting so that we can be most productive moving forward with the docs. I think I can do that in this PR too.

astatt commented 4 years ago

Another small comment: I find it convenient if the sub-topics are all in one big page and you don't have to click on the name of the module to go to its documentation. But it might be because I am used to that style of docs from hoomd. Do you have an opinion on one or the other style?

mphoward commented 4 years ago

I don't quite like that style, which is why I tried this one instead. That breakdown works nicely if you just want to document one module at a time, but it doesn't work so well if you have multiple modules grouped together as a concept (e.g., the potentials are in multiple modules). I also think the pages also get very unmanageable to navigate if you have too many objects all on the same one.

The way this is structured, each concept page will import the module-level documentation (which we need to write, since most are missing docstrings) summarizing the high-level detail of what is in the module and then list the classes that are in the module as a table. The classes in the table are then autogenerated as stub files. It's easy to hop back up to the summary page using the links at the top of each page.

I rarely just "browse" the detailed documentation of all classes (I prefer to browse the summary table instead), which is the main reason I picked this layout.

astatt commented 4 years ago

That's fine, I don't have a strong preference. Let's keep the current layout and fill in the higher level documentation.

mphoward commented 4 years ago
mphoward commented 4 years ago

To avoid too many future conflicts, I think I am going to merge this after PR #30, and we can punt on improving and hosting the docs until later. Is this OK with you?

astatt commented 4 years ago

Yes, sounds good to me, especially the hosting.

mphoward commented 4 years ago
mphoward commented 4 years ago

@astatt I think this is up-to-date enough for us to merge, and we can expand the rest at a later time. Let me know if you agree!

astatt commented 4 years ago

Yeah, I think this looks good enough. We might need to periodically go through and make sure our documentation is updated and accurate, maybe we can agree on doing a check every time before we tag a new version as part of the process?

Ideally, every developer should keep their things up to date, but in practice I think some additional manual checks will be needed from time to time.

mphoward commented 4 years ago

Yes, this is a good idea. I think a few things that we should do are:

  1. Create a PR template (#3) that includes directions for authors to check that they've linked in their new feature to the docs correctly, and that they've checked that it renders correctly.
  2. Add a doc build step to the CI pipeline so that changes do not break the doc build.
  3. Connect the documentation with readthedocs so that it's easy to check different versions of it.
  4. Create an issue template (#3) for tagging a release that includes things like checking the changelog, docs, etc.

Some of these steps are necessary to fully complete #1, but I want to merge this PR now and complete the documentation by v1.0. I'll copy these notes to those issues.