Add inline and Tutorial documentation for internal_coords; change edr…

[rob-miller]

Herewith added tutorial, in-line documentation, and NEWS entry for the internal-coords module work in #3774.

• I hereby agree to dual licence this and any previous contributions under both
  the Biopython License Agreement AND the BSD 3-Clause License.

• I have read the CONTRIBUTING.rst file, have run pre-commit
  locally, and understand that continuous integration checks will be used to
  confirm the Biopython unit tests and style checks pass with these changes.

• I have added my name to the alphabetical contributors listings in the files
  NEWS.rst and CONTRIB.rst as part of this pull request, am listed
  already, or do not wish to be listed. (This acknowledgement is optional.)

The tutorial examples are mostly as doctest code. Also some IMHO minor code changes:

• Change the .aks attribute of class edra (and thus children hedron and dihedron) to .atomkeys as discussed in codespell blocking documentation commit on attribute name #3888. I'm going to punt here, request forgiveness later, and claim the attribute was not well documented enough / code not used enough for this to affect anyone, because it is the right thing to do and avoids tweaking the codespell parameters.
• make the ic_rebuild.structure_rebuild_test results with the -quick option better match the results without that flag (in particular for structures in the Tests/PDB directory).

[codecov]

Codecov Report

Base: 0.00% // Head: 0.00% // No change to project coverage 👍

Coverage data is based on head (cccede9) compared to base (e1902d1).
Patch has no changes to coverable lines.

❗ Current head cccede9 differs from pull request most recent head bcab729. Consider uploading reports for the commit bcab729 to get more accurate results

Additional details and impacted files

@@      Coverage Diff       @@
##   master   #3898   +/-   ##
==============================
==============================

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[rob-miller]

Unable to work out the issue running the tutorial tests on linux. Saw same problem on MacOS and solved by wrapping some evaluations in print() statements, but my Ubuntu 18.04 linux does not show any issues for run_tests.py --offline test_Tutorial.py with python 3.7 and/or 3.8.

@peterjc any insights?

[rob-miller]

Had issues with the following doctests under linux:

>>> myChain.atom_to_internal_coordinates(verbose=True)
chain break at THR  186  due to MaxPeptideBond (1.4 angstroms) exceeded
chain break at THR  216  due to MaxPeptideBond (1.4 angstroms) exceeded

The 'chain break' messages are print()ed by a subroutine a few calls down the stack. Windows and Mac see them, but Linux environment does not. The subroutine appears to work because various results are checked in the subsequent code.

>>> resultDict = structure_rebuild_test(myChain)
>>> resultDict["pass"]
True

On Mac and Windows the result is True as expected, but on Linux it is False. Same code in unittest (test_1a8o) passes fine in all three environments.

>>> myChain2.internal_to_atom_coordinates()
>>> ## confirm result atomArray matches original structure:
>>> np.allclose(cic2.atomArray, myCic.atomArray)
True

Same again - Linux returns False, but the same code in unittest test_1a8o passes fine.

Conclusion: some math is broken in Linux doctest environment?

[peterjc]

This next block of code is just an RST literal.

Were there complications using Python syntax highlighting?

[rob-miller]

No problems, the code is pretty much identical to what's in the Tutorial (except better annotated there). I'm just cynical and expect no one will look past the API docs so wanted the sample code there at the top of the main internal coords file.

[peterjc]

As to the oddity with Linux from this:

% print(chain break message) from subroutine not seen in linux doctest
%xcont-doctest
\begin{minted}{pycon}
>>> # compute bond lengths, angles, dihedral angles
>>> myChain.atom_to_internal_coordinates(verbose=True)
chain break at THR  186  due to MaxPeptideBond (1.4 angstroms) exceeded
chain break at THR  216  due to MaxPeptideBond (1.4 angstroms) exceeded
\end{minted}

That's all via https://github.com/biopython/biopython/blob/master/Tests/test_Tutorial.py which is my glorious hack combining LaTeX with doctests. Without digging into it, my guess might be some code is not using the print(...) function but messing about with sys.stdout instead?

I'm happy with the workaround (skipping that part as a doctest).

[rob-miller]

That's all via https://github.com/biopython/biopython/blob/master/Tests/test_Tutorial.py which is my glorious hack combining LaTeX with doctests. Without digging into it, my guess might be some code is not using the print(...) function but messing about with sys.stdout instead?

And fine and much valued work that (LaTeX + doctests) is!

I tried working on that specific message without improvement, and now hitting the _aligners import issue raised in #3876.

[peterjc]

Can I leave this to you please @JoaoRodrigues since you handled #3774?

[JoaoRodrigues]

Hi @peterjc, sure. On it.

[rob-miller]

Thank you @JoaoRodrigues!!! You are a star and hero for making it so the rest of us can submit pull requests, well done on the investigation and solution.

[rob-miller]

@peterjc have you noticed this problem on the ci tests?

Traceback (most recent call last):
  File "D:\a\biopython\biopython\Tests\test_PhyloXML.py", line 574, in test_phylo
    self._rewrite_and_call(
  File "D:\a\biopython\biopython\Tests\test_PhyloXML.py", line 512, in _rewrite_and_call
    getattr(inst, test)()
  File "D:\a\biopython\biopython\Tests\test_PhyloXML.py", line 287, in test_Distribution
    self.assertEqual(dist.desc, desc)
AssertionError: 'ETH Zürich' != 'ETH Z�rich'
- ETH Zürich
?      ^^
+ ETH Z�rich
?      ^

[peterjc]

I think I saw that last week but haven't opened an issue yet (too busy with a sick child off school), I suspected something changed in the test environment rather than our code. Would you log this please?

[rob-miller]

This is not a WIP, honest -- I'm just using 'in anger' now and finding niggles.

[peterjc]

I do like pretty pictures for documentation :)

[rob-miller]

closes #4213

[peterjc]

@JoaoRodrigues do you still want to review this pull request from Rob? If not there are a few other regular contributors to the PDB code we could ask.

[JoaoRodrigues]

Hey Peter, I'm on it, almost done with it after this last weekend.

[JoaoRodrigues]

I'm gonna go ahead and give this a go, since it's been a while and all tests and documentation seem in good shape. I had some comments but overall, it was nitpicking that's truly not necessary for the PR to be merged.

[peterjc]

Great, thank you.

Rob, it looks like you've deliberately tried to give a useful commit history, so shall we rebase-and-merge this then?

[rob-miller]

Yes please, should go easily as I rebased just 2 weeks ago.

…

On Tue, Jan 24, 2023 at 1:56 PM Peter Cock ***@***.***> wrote: Great, thank you. Rob, it looks like you've deliberately tried to give a useful commit history, so shall we rebase-and-merge this then? — Reply to this email directly, view it on GitHub <#3898 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZW3QFXMJHW3Z2WBCEMK7DWUAJTZANCNFSM5SWCSU6A> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[rob-miller]

Or are you asking me to rebase? I suspect any change from me now breaks Joao's approval?

…

On Tue, Jan 24, 2023 at 2:03 PM Rob Miller ***@***.***> wrote: Yes please, should go easily as I rebased just 2 weeks ago. On Tue, Jan 24, 2023 at 1:56 PM Peter Cock ***@***.***> wrote: > Great, thank you. > > Rob, it looks like you've deliberately tried to give a useful commit > history, so shall we rebase-and-merge this then? > > — > Reply to this email directly, view it on GitHub > < #3898 (comment)>, > or unsubscribe > < https://github.com/notifications/unsubscribe-auth/AAZW3QFXMJHW3Z2WBCEMK7DWUAJTZANCNFSM5SWCSU6A > > . > You are receiving this because you authored the thread.Message ID: > ***@***.***> > — Reply to this email directly, view it on GitHub <#3898 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZW3QH7ZUK5GR4EEUBAZTDWUAKNLANCNFSM5SWCSU6A> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

[peterjc]

Github's rebase-and-merge button was green, it normally warns if a conflict would prevent using it. Clicked!

Thank you again for your patience Rob 👍

[peterjc]

Looking at this afresh, is there anything you'd like to move about in the NEWS file?

[rob-miller]

Looking at this afresh, is there anything you'd like to move about in the NEWS file?

Thank you, yes have submitted #4222 with just that part. Feel free to modify as you see fit.