theochem / procrustes

Python library for finding the optimal transformation(s) that makes two matrices as close as possible to each other.
https://procrustes.qcdevs.org/
GNU General Public License v3.0
109 stars 20 forks source link

Update #68

Closed FarnazH closed 3 years ago

FarnazH commented 3 years ago
  1. The symmetric Procrustes documentation & arguments are finalized.
  2. I have removed References from the docstring because it was making docstring long, and with adding Table 1 (from the paper which contains references) to our documentation, I felt this is redundant. Also, one less thing to keep updated. Let me know what you think @fwmeng88 and @PaulWAyers.
PaulWAyers commented 3 years ago

I'm happy with putting references centrally. I don't worry so much about keeping them up-to-date, or having long documentation (especially if the references are just at the very end anyway), since most of the references we should be including are just "the classics." But I know that because our documentation is embedded in the code, it can be annoying to have a lot of documentation before the code even starts.

I guess the question, then, is whether it is good to have the specific reference where a reader can get more information about the underlying algorithm/method and its justification. I would tend towards including references in the code I guess (where one might wonder what's going on) but as long as there is a source for the references, it is OK I think.

FanwangM commented 3 years ago

We can add a table in About section to summarize all the Procrustes methods (I can take care of it), as we did in the manuscript. But I think adding the references within the codes can help people know exactly what's going on and what are the sources for one specific Procrustes problem.

FarnazH commented 3 years ago

Only 4 of the Procrustes functions had "References" and their formats were not consistent, so I thought it's best to remove them. We can add them later when the summary table is added and our list of references is finalized. We should use the same format for adding references to the docstrings. I will make an issue so we wouldn't forget. Is that fine with you?

FanwangM commented 3 years ago

The reference is not a big problem and I have merged this PR. Many thanks for making the package much nicer! @FarnazH