Skip to content
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

API typos, outdated info, broken links, etc (ongoing) #2401

Open
lilyminium opened this issue Nov 14, 2019 · 5 comments
Open

API typos, outdated info, broken links, etc (ongoing) #2401

lilyminium opened this issue Nov 14, 2019 · 5 comments

Comments

@lilyminium
Copy link
Contributor

@lilyminium lilyminium commented Nov 14, 2019

  • Bond.__eq__ does not check bond order in core.topologyobjects:

    Two :class:`Bond` instances can be compared with the ``==`` and
    ``!=`` operators. A bond is equal to another if the same atom
    numbers are connected and they have the same bond order. The
    ordering of the two atom numbers is ignored as is the fact that a
    bond was guessed.

    def __eq__(self, other):
    """Check whether two bonds have identical contents"""
    if not self.universe == other.universe:
    return False
    return (np.array_equal(self.indices, other.indices) or
    np.array_equal(self.indices[::-1], other.indices))

  • All the "Reads the following Attributes" and "Guesses the following Attributes" should be double-checked in topology.__init__

  • dist is missing documentation of the box parameter (analysis.distances.dist). Note that AtomGroups can be (usually are?) from different Universes. Note that even if the Universe has dimensions associated, dist does not use them unless explicitly passed into box

  • Add lines to initialise k and dist in distances.self_distance_array returns example

  • Update Molnums documentation, which is identical to Moltypes

  • All of the helanal documentation. Best to leave this after #2452 is addressed

  • #2395 fix up guess_bonds documentation

  • All of the PSA documentation. Perhaps it will get AnalysisBased?

  • encore.hes returns unreadable details which are not documented

  • encore.ClusteringMethod.KMeans documents precompute_distances without actually having it as a kwarg

  • encore.DimensionalityReductionMethod.StochasticProximityEmbeddingNative documents the default number of dimensions as 3, but it's actually 2

  • for encore.hes, ces, dres: document that the method returns averages and standard deviations if estimate_error=True

  • for encore.ces_convergence document the output. The shape does not include preference_values.

  • for rdf.InterRDF_s document density kwarg

  • fix base.AnalysisFromFunction raises ValueError

  • document kwargs in Ramachandran.plot

  • PCA.mean_atoms is not the mean positions, PCA.mean is

@richardjgowers

This comment has been minimized.

Copy link
Member

@richardjgowers richardjgowers commented Nov 18, 2019

Yep. So something pandas does is to mush around doc strings with decorators, so we could even do that with topology.__init__ so that when Topology Parsers are defined, they append to that table, so they're automagically done. Or just write some more rst if you don't want to do it programmatically.

@lilyminium

This comment has been minimized.

Copy link
Contributor Author

@lilyminium lilyminium commented Nov 19, 2019

The recently-merged #2390 was created to make a script that auto-docs parsers and the attributes associated. It's currently in the UserGuide but I could move it over to the main repo?

@orbeckst

This comment has been minimized.

Copy link
Member

@orbeckst orbeckst commented Nov 20, 2019

@lilyminium does the UG have a list of all pre-defined topology attributes from core.topologyattrs? Would be useful for Universe.empty() trickery (as I found out when I wasted a few minutes answering a 4-year old Stackoverflow question How to instantiate Atoms using MDAnalysis?).

EDIT: Yes – at https://www.mdanalysis.org/UserGuide/topology_system.html#topology-attributes

@lilyminium

This comment has been minimized.

Copy link
Contributor Author

@lilyminium lilyminium commented Dec 28, 2019

This example in rms.RMSF doesn't work, as align.AlignTraj requires a reference. Also, could use a trajectory that doesn't need to be made whole. And should either explain what order='afc' means or leave it out or use order='fac'; a 1-frame trajectory doesn't need reshaping.

Edit: as pointed out by @jbarnoud, it is easy to forget to clean or preprocess your trajectories before analysis. In that case, putting in the #TODO is now on TODO list 😁

import MDAnalysis as mda
from MDAnalysis.analysis import align

from MDAnalysis.tests.datafiles import TPR, XTC

u = mda.Universe(TPR, XTC, in_memory=True)
protein = u.select_atoms("protein")

# 1) need a step to center and make whole: this trajectory
#    contains the protein being split across periodic boundaries
#
# TODO

# 2) fit to the initial frame to get a better average structure
#    (the trajectory is changed in memory)
prealigner = align.AlignTraj(u, select="protein and name CA", in_memory=True).run()

# 3) reference = average structure
reference_coordinates = u.trajectory.timeseries(asel=protein).mean(axis=1)
# make a reference structure (need to reshape into a 1-frame "trajectory")
reference = mda.Merge(protein).load_new(
            reference_coordinates[:, None, :], order="afc")
@jbarnoud

This comment has been minimized.

Copy link
Contributor

@jbarnoud jbarnoud commented Jan 2, 2020

Also, could use a trajectory that doesn't need to be made whole.

Not dealing with PBC is a frequent misuse of the analysis tools. Showing it has to be done seems a good idea to me. Our examples will be used more or less verbatim in papers.

@lilyminium lilyminium changed the title API typos, outdated info, broken links, etc (ongoing) API typos, outdated info, broken links, formatting, etc (ongoing) Jan 12, 2020
@lilyminium lilyminium changed the title API typos, outdated info, broken links, formatting, etc (ongoing) API typos, outdated info, broken links, etc (ongoing) Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.