Minor fixes #1247

jbarnoud · 2017-03-19T12:48:20Z

This PR is the result of some heavy procrastination. It fixes some minor issues in the package:

Remove the windows end of line in the XVG aux reader
Replace all occurences of isinstance(something, int) by the more generic and safe isinstance(something, numbers.Integral)
Update most docstrings to the numpy format; only the docstrings from the analysis are left.

Please, do not squash the commits as they are unrelated logical units.

kain88-de · 2017-03-19T13:02:32Z

package/MDAnalysis/lib/util.py

+        dict
+            dict with totallength (in chars), repeat, length, format, decimals
+
+        Raises


I think there is a line of dashes missing

kain88-de · 2017-03-19T13:04:42Z

package/MDAnalysis/auxiliary/XVG.py

+        Parameters
+        ----------
+        i : int
+            Step number (0-indexed) to move to


return value doc is missing

richardjgowers · 2017-03-19T22:15:00Z

package/MDAnalysis/auxiliary/XVG.py

@@ -19,343 +19,348 @@
 # MDAnalysis: A Toolkit for the Analysis of Molecular Dynamics Simulations.
 # J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787
 #
-


@jbarnoud is this just rewritten with unix \n instead of whatever MS encoding is?

I just removed the \r at the end of the lines. The file seems to be ASCII as it should be.

orbeckst · 2017-03-21T01:01:06Z

When I build I get a whole bunch of SEVERE errors with messed up docs:

/Volumes/Data/oliver/Biop/Library/python/mdanalysis/package/MDAnalysis/topology/ExtendedPDBParser.py:docstring of MDAnalysis.topology.ExtendedPDBParser:24: SEVERE: Unexpected section title.

Classes
-------
/Volumes/Data/oliver/Biop/Library/python/mdanalysis/package/MDAnalysis/topology/PDBQTParser.py:docstring of MDAnalysis.topology.PDBQTParser:24: SEVERE: Unexpected section title.

Classes
-------
/Volumes/Data/oliver/Biop/Library/python/mdanalysis/package/MDAnalysis/topology/PQRParser.py:docstring of MDAnalysis.topology.PQRParser:4: ERROR: Unknown target name: "pqr".
/Volumes/Data/oliver/Biop/Library/python/mdanalysis/package/MDAnalysis/topology/PQRParser.py:docstring of MDAnalysis.topology.PQRParser:4: ERROR: Unknown target name: "pdb2pqr".
/Volumes/Data/oliver/Biop/Library/python/mdanalysis/package/MDAnalysis/topology/PrimitivePDBParser.py:docstring of MDAnalysis.topology.PrimitivePDBParser:21: SEVERE: Unexpected section title.

Classes
-------
/Volumes/Data/oliver/Biop/Library/python/mdanalysis/package/MDAnalysis/topology/TOPParser.py:docstring of MDAnalysis.topology.TOPParser:28: ERROR: Unknown directive type "todo".

.. todo:: Add reading of bonds etc
/Volumes/Data/oliver/Biop/Library/python/mdanalysis/package/MDAnalysis/topology/TPRParser.py:docstring of MDAnalysis.topology.TPRParser:73: SEVERE: Unexpected section title.

Development notes
-----------------
/Volumes/Data/oliver/Biop/Library/python/mdanalysis/package/MDAnalysis/topology/TPRParser.py:docstring of MDAnalysis.topology.TPRParser.TPRParser:1: ERROR: Unknown target name: "tpr".
/Volumes/Data/oliver/Biop/Library/python/mdanalysis/package/MDAnalysis/topology/__init__.py:docstring of MDAnalysis.topology.__init__:147: SEVERE: Unexpected section title.

Developer Notes
---------------
/Volumes/Data/oliver/Biop/Library/python/mdanalysis/package/MDAnalysis/visualization/streamlines.py:docstring of MDAnalysis.visualization.streamlines.generate_streamlines:65: WARNING: Explicit markup ends without a blank line; unexpected unindent.

Typically, this seems to be something like

See Also
--------
* item 1
* item 2

Classes
-------

.. autoclass:: Foo

At least my version of sphinx (v1.5.1)/napoleon seems to be unhappy about the numpy-style "See Also" because when I change it back to

.. SeeAlso::
    * item 1
    * item 2

Classes
-------

.. autoclass:: Foo

then everything is fine again.

Is this just me?

(Generally speaking, the numpy docs format does some hackish things such as any heading "Examples" is turned into a non-section heading and the "See Also" probably also only works nicely inside class/method/function docs.)

jbarnoud · 2017-03-21T10:16:35Z

The easiest is probably to revert back to the old .. SeeAlso::. I'll do it as soon as I find some time.

jbarnoud · 2017-03-22T21:51:31Z

It seems that I actually broke quite a bit of the doc, and that fixing it is not trivial. I'll try to solve that this weekend.

orbeckst

Some of the formatting got broken.

See, for instance, problems with See Also.

I am blocking the merge until fixed.

orbeckst · 2017-04-21T19:33:58Z

This will need to be rebased after PR #1314 will have been merged.

orbeckst · 2017-04-27T08:16:01Z

@jbarnoud do you want to try and rebase and hopefully get it in soon?

jbarnoud · 2017-04-27T08:40:55Z

I plan on going back to it next week.

orbeckst · 2017-05-01T21:34:49Z

@jbarnoud I rebased against develop and fixed the conflicts. Please have a look.

(Note: this needs to be merged.)

Testing that arguments where integer by using `isinstance(arg, int)` has proven itself detrimental on several occasions. Indeed, the test refuses other types that would be legitimate such as numpy integers. This commit replaces the remaining occurences of the too narrow test by the solution that has already been used on multiple places of the code: testing against `numbers.Integral`. This new way matches everything that looks enough like an integer, and behave the same on python 2 and python 3 on the contrary to testing against `int`.

Update the docstrings everywhere in the package but in the anlysis.

…efile) Normally we build the docs with python setup.py build_sphinx but for more fine-grained control one can also locally build with cd doc/sphinx make html This is useful for debugging reST issues.

- I fixed everything until local cd doc/sphinx make html passed without errors.... but I am sure that more errors will come up. - reformatted and rewrote various portions of the docs while fixing the errors - completed the ENCORE citations in encore/similarity.py

orbeckst · 2017-05-05T01:23:24Z

If this passes Travis then I need another review, please.

Fixed remaining issues.
rebased against develop and cleaned up.

orbeckst · 2017-05-05T01:24:52Z

Note: Merge please see #1247 (comment) (do not squash)

jbarnoud · 2017-05-05T11:06:28Z

I fixed the last new rendering error I could find. It is good to go on my side.

kain88-de · 2017-05-05T13:53:55Z

package/MDAnalysis/auxiliary/base.py

 import math
 import warnings

 from ..lib.util import asiterable, anyopen
+import numpy as np


the numpy import should still be independent from the mdanalysis internal imports

kain88-de · 2017-05-05T13:55:38Z

package/MDAnalysis/coordinates/DCD.py

-
+    """Write to a CHARMM/NAMD DCD trajectory file.
+
+    Parameters


I thought we decided to put those into the init function

Honestly, I don't remember a discussion, I might have missed that. I don't see anything in the Style Guide: Docs.

My reasoning was that with the extended numpy sections it looks better to have everything in the class docs instead of __init__, especially as sometimes we might not even have __init__ for derived classes.

I certainly followed this reasoning in some of the analysis module doc rewrites where class docs contained background and examples and __init__ contained parameter lists.

Can you point me to where we decided using __init__ for the class docs? For right now I'd like to leave it as it is.

Not sure which PR anymore. But we currently have both styles in the library.

I'd say: open an issue to change it to one or the other and update the style guide according to the consensus.

kain88-de · 2017-05-05T13:56:07Z

package/MDAnalysis/coordinates/DCD.py

@@ -391,15 +401,15 @@ class DCDReader(base.ReaderBase):
        ``data = dcd.correl(...)``
           populate a :class:`MDAnalysis.core.Timeseries.Collection` object with computed timeseries

-    .. Note::
+    Note
+    ----



I think the empty line here should be removed.

Done (empty line not harmful and positively helps when reformatting the block in emacs but I agree that this should be consistent everywhere)

kain88-de · 2017-05-05T13:56:24Z

package/MDAnalysis/coordinates/DCD.py

-                Skip has been deprecated in favor of the standard keyword step.
+        Parameters
+        ----------
+        asel: :class:`~MDAnalysis.core.groups.AtomGroup`


missing space after asel

done (not harmful, but yes, let's keep the style uniform)

... and all the other occurences in DCD.py

kain88-de · 2017-05-05T13:56:59Z

package/MDAnalysis/coordinates/DCD.py

+
+        Returns
+        -------
+        :class:`DCDWriter`

        .. Note::


please change the Note section here as well.

kain88-de · 2017-05-05T14:01:07Z

package/MDAnalysis/lib/util.py

+        :attr:`NamedStream.name` will be changed accordingly)
+    ext: None or str
+        extension to use in the new filename
+    keep: bool


missing space

kain88-de · 2017-05-05T14:01:16Z

package/MDAnalysis/lib/util.py

+    mode: str
+        'r' or 'w' or 'a', more complicated modes ('r+', 'w+' are not supported
+        because only the first letter is looked at)
+    reset: bool


missing space

kain88-de · 2017-05-05T14:01:27Z

package/MDAnalysis/lib/util.py

+    Returns
+    -------
+    root: str
+    ext: str


missing space

kain88-de · 2017-05-05T14:01:40Z

package/MDAnalysis/lib/util.py

            last column + 1
-         *typespecifier*
+        typespecifier: str


missing space

kain88-de · 2017-05-05T14:01:53Z

package/MDAnalysis/lib/util.py

-               allowed in between.
+    Parameters
+    ----------
+    residue: str


missing space

orbeckst · 2017-05-05T21:43:10Z

I think I addressed @kain88-de 's comments (and more...).

Fixing docs is a bottomless pit for time and I spent way more time on this than I should have. I will fix anything in my commits that has broken something but for any other wishes, please do them yourself on the PR.

orbeckst · 2017-05-07T00:14:45Z

I found that many coordinate classes were not even shown in the docs so I added explicit .. autoclass markup where necessary. Also fixed the link to AtomGroup that @kain88-de pointed out (thanks).

I rebased into my last commit to keep the history tidy and immediately mergable. Force-pushed... and hopefully done.

@kain88-de

- @kain88-de suggestions - even more fixes (everywhere in coordinates, elsewhere where I found something to fix). In particular - adde Parameter/Return/Raises sections to many functions in lib.util - made sure that there were two empty lines above .. versionchanged etc sections - made proper Example sections - fixed/clarified text - ensure that *all* classes are shown and can be linked (added autoclass markup where necessary) - PDB: removed any mentioning of the simple/Bio.PDB parser stuff, only left a versionchanged entry

orbeckst · 2017-05-07T08:06:20Z

Why does the numpy dev test fail?? https://travis-ci.org/MDAnalysis/mdanalysis/jobs/229565210 – it looks as if it sets up the environment and then just dies silently.

I think I don't understand

how "cron" tests are supposed to work
if (and how) the astropy-ci helper scripts handle numpy dev

kain88-de · 2017-05-07T09:16:03Z

Why does the numpy dev test fail??

see Discussion at #1334

orbeckst · 2017-05-07T09:18:28Z

Thanks!

kain88-de

I assume everything is correct. I haven't checked every line of this massive PR

kain88-de · 2017-05-07T17:54:10Z

package/doc/sphinx/Makefile

@@ -3,7 +3,7 @@

 # You can set these variables from the command line.
 SPHINXOPTS    =
-SPHINXBUILD   = sphinx-build
+SPHINXBUILD   = sphinx-build -v -W


what are these options doing?

Verbose (more output) and make warnings failure (the same thing that you (?) enabled in the setup.cfg file).

Running

python setup.py build_sphinx

is apparently not using this Makefile but if one wants to reproduce locally the build with the Makefile then I found it useful to have. I didn't know how to change any other build_sphinx option for testing so it was convenient to use the standard Makefile.

orbeckst · 2017-05-07T19:43:05Z

I merged it as is and ignored the travis failures.

orbeckst · 2017-05-07T19:43:47Z

Thanks everyone for these "minor fixes"... especial thanks to @kain88-de for repeatedly reviewing.

jbarnoud added the needs review label Mar 19, 2017

kain88-de reviewed Mar 19, 2017

View reviewed changes

jbarnoud force-pushed the minor-fixes branch from 78cf109 to fc52bf2 Compare March 19, 2017 13:13

richardjgowers reviewed Mar 19, 2017

View reviewed changes

richardjgowers previously approved these changes Mar 19, 2017

View reviewed changes

orbeckst self-requested a review March 23, 2017 06:04

orbeckst requested changes Mar 23, 2017

View reviewed changes

orbeckst mentioned this pull request Mar 23, 2017

transition analysis docs to numpy style #1240

Merged

1 task

orbeckst added Component-Core Component-Docs labels Mar 23, 2017

orbeckst self-assigned this Mar 27, 2017

orbeckst mentioned this pull request Apr 1, 2017

convert all docs to numpy style #1277

Closed

3 tasks

orbeckst mentioned this pull request Apr 17, 2017

fix sphinx warnings #1312

Merged

4 tasks

orbeckst previously approved these changes Apr 21, 2017

View reviewed changes

orbeckst force-pushed the minor-fixes branch from 4d4c063 to b889372 Compare May 1, 2017 21:34

orbeckst dismissed stale reviews from richardjgowers and themself via 608e7ee May 5, 2017 01:16

jbarnoud and others added 6 commits May 4, 2017 18:18

Remove windows end of lines

a8ea036

Numpyfy docstrings

a8f5157

Update the docstrings everywhere in the package but in the anlysis.

Update guessers.py

2793519

sphinx: make sphinx fail on warnings and be verbose (local sphinx Mak…

e25d2c7

…efile) Normally we build the docs with python setup.py build_sphinx but for more fine-grained control one can also locally build with cd doc/sphinx make html This is useful for debugging reST issues.

orbeckst force-pushed the minor-fixes branch from 608e7ee to 4f3d59c Compare May 5, 2017 01:22

orbeckst added this to the 0.16.x milestone May 5, 2017

orbeckst added the refactoring label May 5, 2017

Fix small rendering errors in the doc

7817f96

kain88-de reviewed May 5, 2017

View reviewed changes

orbeckst force-pushed the minor-fixes branch from dd1c97f to 4104ae4 Compare May 7, 2017 00:12

orbeckst force-pushed the minor-fixes branch from 4104ae4 to ecc3919 Compare May 7, 2017 00:19

orbeckst removed their assignment May 7, 2017

kain88-de previously approved these changes May 7, 2017

View reviewed changes

use consistent formatting

c1db033

kain88-de dismissed their stale review via c1db033 May 7, 2017 18:10

kain88-de approved these changes May 7, 2017

View reviewed changes

orbeckst merged commit d5667ae into develop May 7, 2017

orbeckst deleted the minor-fixes branch May 7, 2017 19:43

Minor fixes #1247

Minor fixes #1247

Conversation

jbarnoud commented Mar 19, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orbeckst commented Mar 21, 2017

jbarnoud commented Mar 21, 2017

jbarnoud commented Mar 22, 2017

orbeckst left a comment

Choose a reason for hiding this comment

orbeckst commented Apr 21, 2017

orbeckst commented Apr 27, 2017

jbarnoud commented Apr 27, 2017

orbeckst commented May 1, 2017

orbeckst commented May 5, 2017

orbeckst commented May 5, 2017 • edited Loading

jbarnoud commented May 5, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orbeckst commented May 5, 2017

orbeckst commented May 7, 2017

orbeckst commented May 7, 2017

kain88-de commented May 7, 2017

orbeckst commented May 7, 2017

kain88-de left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

orbeckst commented May 7, 2017

orbeckst commented May 7, 2017

orbeckst commented May 5, 2017 •

edited

Loading