-
Notifications
You must be signed in to change notification settings - Fork 647
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 for many AtomGroup methods missing #1845
Comments
I don't know how to fix this unless we explicitly write the docs for AtomGroup. @richardjgowers @kain88-de @dotsdl any ideas (eg from datreant) how to properly document when you're doing this magic patching of groups? From a user's point of view it is absolutely not comprehensible why certain commonly used methods are not documented at all. And it makes it hard to answer user questions because I cannot just link to the docs but have to say "there's this method that just happens to come into existence when you have masses but it's kind of a secret so you need to know about it...". |
We might be able to hack it in in the rst docs by manually listing them.
If not I'm happy to move them into the class and have them fail gracefully
without masses
…On Sun, 25 Mar 2018, 13:27 Oliver Beckstein, ***@***.***> wrote:
I don't know how to fix this unless we explicitly write the docs for
AtomGroup. @richardjgowers <https://github.com/richardjgowers> @kain88-de
<https://github.com/kain88-de> @dotsdl <https://github.com/dotsdl> any
ideas (eg from datreant) how to properly document when you're doing this
magic patching of groups?
From a user's point of view it is absolutely not comprehensible why
certain commonly used methods are not documented at all. And it makes it
hard to answer user questions because I cannot just link to the docs but
have to say "there's this method that just happens to come into existence
when you have masses but it's kind of a secret so you need to know about
it...".
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1845 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AI0jB3eK1yRGuyugWQdAcCpC0KcX5eLOks5th-GOgaJpZM4S6Pt0>
.
|
@richardjgowers What needs to be done to get these docs back? If it's rst hacking then if you show one or two examples then I can do some more. A todo list would also be helpful... or perhaps we can generate the list/rst automatically? |
Ping @richardjgowers – if you give me some starters I can chip away on this one. I really want proper docs for 0.19.0! |
@orbeckst I'm not sure the easiest way to solve this one. The fancy "attaching methods on the fly with attributes" doesn't really lend itself well to how we automatically generate docs. The best solution would be to write the docs manually, (or at least list the methods in order), this would have the benefit that we could control the order & presentation of them ## Position related methods
Blah blah blah, these all take common kwargs like `pbc`, this keyword always does xyz
.. automethod:: `MDAnalysis.core.groups.center`
.. automethod:: `MDAnalysis.core.groups.center_of_geometry`
.. automethod:: `MDAnalysis.core.topologyattrs.Masses.center_of_mass`
.. automethod:: `MDAnalysis.core.topologyattrs.Masses.radius_of_gyration`
|
Ok, then we manually write the docs and add a comment to the code that says that you have to add the appropriate doc entry there and there. |
... or we write a small python script that writes the list for us :-). u = Universe("full_featured_topology.tpr")
methods = dir(u.atoms)
for method in methods:
if str(method).startswith("_"):
continue
print(".. automethod:: {}".format(str(method))) ... something along those lines |
Mouahahahahaha! import collections
import MDAnalysis as mda
targets = collections.defaultdict(lambda : collections.defaultdict(list))
for attribute_key, attribute in mda.core.topologyattrs._TOPOLOGY_ATTRS.items():
for target, methods in attribute.transplants.items():
for method in methods:
#print(method)
function = method[1]
try:
# We my be dealing with a property; then we need to get the
# actual method out of it.
function = function.fget
except AttributeError:
# Well, it was not a property
pass
text = '.. automethod:: `{}.{}`'.format(function.__module__, function.__qualname__)
targets[target][attribute_key].append(text)
for target_key, target_dict in targets.items():
try:
target_name = target_key.__name__
except AttributeError:
# For some reason, some target are not classes but str
target_name = target_key
print()
print(target_name)
print('=' * len(target_name))
for attribute_key, method_list in target_dict.items():
print()
print('Requires ' + attribute_key)
print('-' * (len('Requires ') + len(attribute_key)))
print()
for method_text in method_list:
print(method_text) Prints:
Now, is there a way to have sphinx inject it directly in the right docstrings? |
@jbarnoud that looks like it'd be good enough as a rst stub, then sphinx will do its magic when we build docs |
Can we re-label these methods so that it looks as if they came from AtomGroup? .. automethod:: `AtomGroup.center_of_mass <MDAnalysis.core.topologyattrs.Masses.center_of_mass>` should work. We will still have to link to the transplant when referring to the method as in You can compute the center of mass with the :meth:`~MDAnalysis.core.topologyattrs.Masses.center_of_mass` method. but this is already much better than before. There might be a way to write our own sphinx plugin to somehow change things later but for right now I'd be happy with going with @jbarnoud 's script. Can you add it to .. include:: atomgroup_methods.rst (Search through http://docutils.sourceforge.net/docs/ref/rst/directives.txt because the official reST html page on directives is broken.) |
If we want to include the generated file into the docs for .. rubric:: Requires masses and it would also be better to have separate files such as
You can also add a comment at the top .. generated with script maintainer/awesome_method_extractor.py on YYYY-MM-DD
.. commit: deadbeef EDIT: For getting the current commit: import subprocess
rev = subprocess.getoutput("git rev-parse HEAD") |
We are also missing from the containers
|
@orbeckst I'll look at this as soon as I have time. I like the @richardjgowers Do you know if it is possible to distinguish singulars and plurals from |
@jbarnoud iterate over a set of the values of # replaces your first loop
for attribute in set(mda._TOPOLOGY_ATTRS.values()):
attribute_key = attribute.attrname |
Good idea. Thanks.
…On 07/13/2018 07:36 PM, Richard Gowers wrote:
@jbarnoud <https://github.com/jbarnoud> iterate over a set of the
values of |mda._TOPOLOGY_ATTRS| then grab the |attrname| off each TA
# replaces your first loop
for attributein set(mda._TOPOLOGY_ATTRS.values()):
attribute_key= attribute.attrname
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1845 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUWuvfAGw6SvANkr75-VljrRODzJ1Htks5uGNqEgaJpZM4S6Pt0>.
|
The `transplant_stub.py` script introspects the groups and topology attributes to write files in `documentation_pages/core` that contain the documentation for the transplanted methods. For the stubs to be picked up by sphinx, the docstring of the class to document must contain .. include:: XXX.txt where "XXX" is the name of the class. A stub contains a table of the methods, their short descriptions, and what topology attribute they require. Fixes #1845
The `transplant_stub.py` script introspects the groups and topology attributes to write files in `documentation_pages/core` that contain the documentation for the transplanted methods. For the stubs to be picked up by sphinx, the docstring of the class to document must contain .. include:: XXX.txt where "XXX" is the name of the class. A stub contains a table of the methods, their short descriptions, and what topology attribute they require. Fixes #1845
- Fixes #1845 - Adds dummy method with documentation of original method - Dummy method gets replaced by actual method on instantiation
- Fixes MDAnalysis#1845 - Adds dummy method with documentation of original method - Dummy method gets replaced by actual method on instantiation
Expected behaviour
radius_of_gyration(), principal_axes() (see #1828 ), center_of_mass() and most other "compute something on an AtomGroup" (that require e.g., mass) should be in the docs for AtomGroup.
Actual behaviour
They are not in the docs anymore (at least since 0.17.0 but probably since #363 / 0.16.0)
Code to reproduce the behaviour
Look at https://www.mdanalysis.org/mdanalysis/documentation_pages/core/groups.html#MDAnalysis.core.groups.AtomGroup
I think the problem is that we only attached certain methods to AtomGroup when specific TopologyAttrs are present. The bare AtomGroup does not contain masses so many of the most used methods are invisible to the user (and a lot of our docs is broken, e.g., in the tutorial).
Currently version of MDAnalysis:
develop, but also published 0.17.0
The text was updated successfully, but these errors were encountered: