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

MDAnalysis.analysis.distances.contact_matrix #365

Closed
tylerjereddy opened this issue Jul 24, 2015 · 4 comments
Closed

MDAnalysis.analysis.distances.contact_matrix #365

tylerjereddy opened this issue Jul 24, 2015 · 4 comments

Comments

@tylerjereddy
Copy link
Member

The docstring in 0.11 for this function incorrectly suggests that a list of coordinates is used as input, while a numpy array is clearly the correct input (list won't work).

Also, in practice I am not seeing the progress meter before or after 0.11 (mac or linux, even with a large input array), which I was testing for when trying to write some user documentation for 0.11 API updates MDAnalysis 0.11 unifying release user guide: Suppressing contact matrix progress meter.

@dotsdl
Copy link
Member

dotsdl commented Jul 24, 2015

I made changes recently to the ProgressMeter itself; not sure if these are related. What is most recent release in which you see the progress meter for this function?

@tylerjereddy tylerjereddy self-assigned this Jul 25, 2015
@tylerjereddy
Copy link
Member Author

There seem to be a few issues. First, using printf with the C code written inline with scipy.weave uses a different standard out than Python, so it is invisible in the IPython notebook and any other GUI-type IDEs (this is stated in the scipy.weave docs). I'm not sure there's much we can easily do about that, and maybe it is not that important anyway, though I suppose the docstring could be updated to reflect this fact.

Second, the progress will not be printed for the default returntype = "numpy" call signature, presumably because it is a fast calculation for which the progress meter was not thought necessary. The progress will print out with returntype = "sparse", though again only in a terminal window and not in a notebook. My general feeling is that the docs should indicate that the progress meter will not display when using the default returntype, regardless of the setting of the quiet keyword to True or False.

I should also aim to update the wiki 0.11 example above, which is not meaningful because of the default returntype.

@richardjgowers
Copy link
Member

It's not worth putting too much time into fixing the weave code, it's getting replaced soon hopefully #264

@tylerjereddy
Copy link
Member Author

I've updated the docstring of contact_matrix to reflect the proper input data type and also the proper behaviour of the progress meter. After the weave code is replaced, additional changes to the docs will likely be needed, at which stage we should probably adopt the numpy doc standard, but I'm closing this with the minor clarifications that should suffice for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants