-
Notifications
You must be signed in to change notification settings - Fork 40
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
Formats and AtomGroup methods #30
Conversation
If there's a way to run the examples (doctest?) then we would at least know when the user guide becomes outdated. This is an important issue but I don't know how to solve it (certainly not part of this issue). How do matplotlib and scipy (@tylerjereddy ?) solve keeping their docs in sync with code? We could put a mature UserGuide in the main repo but I'd prefer rather moving stuff out of the main repo.... |
I built this PR at https://lilyminium.github.io/UserGuide/ for convenience. |
98e1488
to
09b3989
Compare
@orbeckst @richardjgowers I'd love your thoughts on this PR in its current state. (viewable here) |
One option for keeping things up to date is to rebuild the docs each release, then a quick grep through the repo to check for references to the old and new version? I’d prefer to keep these docs separate |
The auto generated format specific topologyattrs seems to have picked up a few base classes (eg atomattr) |
Thanks for the review, @richardjgowers. I'll merge tomorrow if you and @orbeckst are happy. |
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.
Really awesome work! I only managed to review a small subset and found a few things I would change.
Thanks for the comments @orbeckst. I realise 115 is a lot of files changed and a massive pain to review. (I think script-generated tables make up more than a few of those, and file format stubs). Is there anything I can do to make this easier to review? (Going forward I'll try not to let PR's get away from me like 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.
a few more comments, but I think you can merge and we just keep raising issues when we see something in the product, looks already very good (just ran out of more time to review)
@@ -5,8 +5,8 @@ | |||
/* MDAnalysis gray: #808080 */ | |||
/* MDAnalysis white: #FFFFFF */ | |||
/* MDAnalysis black: #000000 */ | |||
/* Darker orange: e76900 */ | |||
/* Even darker orange: #a24900 */ | |||
/* Very light orange: #FFEBD0 */ |
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.
Are these changes that need to be ported to the MDAnalysis docs, too?
@@ -19,10 +19,12 @@ Atom selection language | |||
|
|||
AtomGroup instances are typically created with :meth:`Universe.select_atoms <MDAnalysis.core.universe.Universe.select_atoms>` or by manipulating another :class:`~MDAnalysis.core.groups.AtomGroup`, e.g. by slicing. | |||
|
|||
.. code-block:: python | |||
.. ipython:: python |
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.
What does an ipython::
block do? Just curious.
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.
Evaluate?
That's cool!
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, ipython:: python
lets you write Python input and IPython will handle the output, generating errors during doc compilation if there are any. Good for handling typos and not copy-paste outputs.
doc/source/conf.py
Outdated
@@ -26,7 +26,7 @@ | |||
|
|||
# -- Scripts ----------------------------------------------------------------- | |||
# regenerate tables for which residues are selected by keywords | |||
subprocess.call('./scripts/gen_standard_selections.py') | |||
# subprocess.call('./scripts/gen_standard_selections.py') |
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.
Maybe comment on how this works or reference a README that says how the docs are build?
If it's not needed then better remove to avoid maintainer confusion.
To do:
- [ ] Have IPython examples if 0.21.0 is released?- [ ] Move scripts toconf.py
for automatic executionRunning the first script currently requires this branch.
Current open question: how to make sure the user guide stays up to date? 0.21.0 will already have significant changes that mean some warnings and notes are out-of-date. The scripts, notebooks, and IPython help to an extent by actually executing MDAnalysis code. Perhaps the user guide should be part of the main repo; then, at least, the examples won't be wrong.