fix #216: angle parameters are not being read from topology file

[jandom]

This will be a fair amount of work actually, to update the parser and support all the different angletypes defined by gromacs... I'm treating this documentation as authoritative source of what to expect for each angletype https://manual.gromacs.org/current/reference-manual/topologies/topology-file-formats.html#tab-topfile2

[jandom]

@orbeckst oh I see that we're still supporting python2.7 in this repo – is that intentional, or do you not actually care?

E     File "/home/runner/work/GromacsWrapper/GromacsWrapper/tests/fileformats/top/test_TOP.py", line 26
E       def create_topology_data() -> str:
E                                  ^
E   SyntaxError: invalid syntax

[orbeckst]

Looking at it, we still do support 2.7 — mainly because it was no real effort to do so. However, that could go by the wayside eventually, but that requires a last release with 2.7, a dedicated cleanup PR etc so for right now, just throw out anything modern and make the tests pass.

(Have a look at #offtopic in the MDAnalysis discord if you want to hear opinions on Python's development cycles...)

[orbeckst]

Please ping me when you need a quick review @jandom . I don't have a lot of free side project time at the moment.

[orbeckst]

And PS: Many thanks for taking on the issue!!!!

[jandom]

No problem! Anybody else that could review the PR in your absence Oli? My one question so far is: do we need to continue and support python 2.7?

On Tue, Nov 7, 2023 at 16:19 Oliver Beckstein @.***> wrote:

And PS: Many thanks for taking on the issue!!!!

— Reply to this email directly, view it on GitHub https://github.com/Becksteinlab/GromacsWrapper/pull/264#issuecomment-1799070784, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADAVG4RE6F2LBW4G3BXDRDYDJNRJAVCNFSM6AAAAAA66JDKQKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTOOJZGA3TANZYGQ . You are receiving this because you were mentioned.Message ID: @.***>

[codecov[bot]]

Codecov Report

Merging #264 (6664d77) into main (fa64735) will increase coverage by 0.29%. The diff coverage is 89.47%.

@@            Coverage Diff             @@
##             main     #264      +/-   ##
==========================================
+ Coverage   68.88%   69.18%   +0.29%     
==========================================
  Files          22       22              
  Lines        3944     3946       +2     
  Branches      708      715       +7     
==========================================
+ Hits         2717     2730      +13     
+ Misses       1047     1036      -11     
  Partials      180      180              

Files	Coverage Δ
gromacs/fileformats/blocks.py	72.35% <100.00%> (+1.38%)	:arrow_up:
gromacs/fileformats/top.py	86.89% <86.04%> (+1.27%)	:arrow_up:

:mega: Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

[orbeckst]

do we need to continue and support python 2.7?

I am looking at #259 where we/I apparently decided to finally drop 2.7. And I even made the 0.8.5 release that was officially the last one to also support "old Python".

Therefore — you'll be overjoyed to hear — we can rip out any (very) legacy Python ≤ 3.7. I would, however, prefer to do the ripping in a separate PR; for a start just removing the testing in CI and updating the CHANGELOG would be sufficient.

[orbeckst]

If you review PR #265 then you can just use Python ≥3.8 in this PR ;-).

[jandom]

If you review PR https://github.com/Becksteinlab/GromacsWrapper/pull/265 then you can just use Python ≥3.8 in this PR ;-).

Approved! But of course I coded first and read second so I already factored out the enum import...

[orbeckst]

Thank you @jandom !!!

[jandom]

Many thanks for your help with this PR @orbeckst – I will note that ChatGPT was heavily used to write this code, with a few adjustments! 🤖

[orbeckst]

I am just relying on you claiming proper authorship of the code and having the knowledge to know when something coming out of a LLM is correct.