TPR reader sets chainID from molblock

[pgbarletta]

Fixes #3994

Trying to revive this issue, following @orbeckst's suggestion (don't set molblock to Segments, but to chainIDs). As usual, I'll delete the PR and the issue if this is not deemed useful.

Changes made in this Pull Request:

• tpr_utils.do_mtop() sets the chainID of each atom as this: chainID = molblock[14:] if molblock[:14] == "Protein_chain_" else molblock
• on the testing side: TPRAttrs adds chainIDs to the list expected_attrs. I didn't thought it was necessary to add more tests on top of that.

If approved I'll add 'chainID' here.

This does not warrant an addition to the versionchanged heading, right?

PR Checklist

• Tests?
• Docs?
• CHANGELOG updated?
• Issue raised/referenced?

Developers certificate of origin

• I certify that this contribution is covered by the LGPLv2.1+ license as defined in our LICENSE and adheres to the Developer Certificate of Origin.

---

📚 Documentation preview 📚: https://mdanalysis--4281.org.readthedocs.build/en/4281/

[github-actions]

Linter Bot Results:

Hi @pgbarletta! Thanks for making this PR. We linted your code and found the following:

Some issues were found with the formatting of your code.

Code Location	Outcome
main package	⚠️ Possible failure
testsuite	✅ Passed

Please have a look at the darker-main-code and darker-test-code steps here for more details: https://github.com/MDAnalysis/mdanalysis/actions/runs/6190054151/job/16805470720

---

Please note: The black linter is purely informational, you can safely ignore these outcomes if there are no flake8 failures!

[orbeckst]

I am in favor of your change, @pgbarletta.

Do the tests check that specific chainIDs were parsed?

Please also do the usual (update CHANGELOG, docs, ...). Ping me when you need me to review again.

[codecov]

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (16647cb) 93.40% compared to head (ea0a06f) 93.40%.

Additional details and impacted files

@@            Coverage Diff             @@
##           develop    #4281     +/-   ##
==========================================
  Coverage    93.40%   93.40%             
==========================================
  Files          170      184     +14     
  Lines        22250    23363   +1113     
  Branches      4071     4071             
==========================================
+ Hits         20783    21823   +1040     
- Misses         951     1024     +73     
  Partials       516      516             

Files Changed	Coverage Δ
package/MDAnalysis/topology/TPRParser.py	90.16% <ø> (ø)
package/MDAnalysis/topology/tpr/utils.py	95.52% <100.00%> (+0.04%)	⬆️

... and 14 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pgbarletta]

I added the expected chainIDs to TestTPRGromacsVersions ---which inherits from TPRAttrs---, and then added the test_chainIDs() function to TPRAttrs. Let me know if I interpreted the testing style correctly or not.

[orbeckst]

Looks good to me. I think the testing is doing the right thing. It's a bit complicated and it took me a while to see what's happening with all the inheritance. The fact that you're testing a case with two chains gives me some confidence that it works.

Sorry for the delay, I had thought I had already done this review but obviously not... in general, feel free to ping me when a week has passed!

[orbeckst]

Thanks @pgbarletta !

[orbeckst]

This does not warrant an addition to the versionchanged heading, right?

I read through everything an decided at the very last minute that it was better to document what we are doing and added it.

[orbeckst]

We do need to update the User Guide so I raised MDAnalysis/UserGuide#312 — could you please do that, @pgbarletta ? Thanks.