First PR was very nice. Is our workflow good/was there enough detail? JM – Ambiguity about whether to update comments – Details felt like restrictions, wasn’t sure when to exercise judgement JW – I’ll provide less details except where I foresee ambiguity/complexity.
Big-picture plans for toolkit showcase? JW – This would be valuable, but I recognize it’s extremely broad in scope JM – Working on it, had to recover a file of utilities that were extracted from original notebook.
Let’s find a medium task in the meantime Sphinx vs. Mkdocs Release notes polishing Just fix small things as you find them? ParameterAttribute docs are notably broken
|
Next tasks: Fix small things as you find them → Avoid cases of ambiguity, JW can do a high PR review throughput on this → best to keep them localized to one/a few files.
Make docs builds stop emitting so many (if possible, “all”) warnings Try converting a few pages of docs to mkdocs – Provide a proof of concept for JW, that we could bring forward to the other core developers if it looks promising. Try to fix ParameterAttributes so that docstrings don’t just repeat the docstring from base ParameterAttribute in the rendered docs (eg the “method” description on this page: https://open-forcefield-toolkit.readthedocs.io/en/0.8.3/api/generated/openforcefield.typing.engines.smirnoff.parameters.vdWHandler.html#openforcefield.typing.engines.smirnoff.parameters.vdWHandler) Constraint: The base ParameterAttribute should have its full docstring in the compiled docs Constraint: This can’t require an explcit removal of the docstring on every parameterattribute (so, we can’t have a line likevdWHandler.method.__docstring__ = ”” , just because it would be added complexity that downstream developers have to deal with) A good solution would be to have the base ParamterAttribute docstring be full in the docs, and all instances have blank docstrings A better solution would be to have an optional keyword argument in the init method that sets a custom docstring which is blank by default, eg
potential = ParameterAttribute(default="Lennard-Jones-12-6", converter=_allow_only(["Lennard-Jones-12-6"], docstring='pick 12-6') |