feature: IMDReader Integration

[amruthesht]

Fixes #4827

This draft PR addresses the feature request discussed in #4827.

Note:
The IMDReader feature which was previously a part of the imdclient package has been moved into MDAnalysis below. Any other modules have been in retained in imdclient, which has been added as an optional dependency here. We are currently in the process of splitting the imdclient package as mentioned above. (Issue, PR)

Major changes made in this Pull Request:

• IMDReader, other associated base classes and a utility function were added to coordinates in the main package.
• Appropriate test cases for the reader were added as a part of test-imd.py
• Other corresponding changes were made to CI and environment *.yaml files
• imdclient was added as an optional dependency

PR Checklist

• Issue raised/referenced?
• Tests updated/added?
• Documentation updated/added?
• package/CHANGELOG file updated?
• Is your name in package/AUTHORS? (If it is not, add it!)

Developers Certificate of Origin

I certify that I can submit this code contribution as described in the Developer Certificate of Origin, under the MDAnalysis LICENSE.

---

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

[github-actions]

Hello there first time contributor! Welcome to the MDAnalysis community! We ask that all contributors abide by our Code of Conduct and that first time contributors introduce themselves on GitHub Discussions so we can get to know you. You can learn more about participating here. Please also add yourself to package/AUTHORS as part of this PR.

[codecov]

Codecov Report

Attention: Patch coverage is 67.25146% with 56 lines in your changes missing coverage. Please review.

Project coverage is 85.84%. Comparing base (c4af00b) to head (27597e7).

Files with missing lines	Patch %	Lines
package/MDAnalysis/coordinates/IMD.py	47.72%	45 Missing and 1 partial ⚠️
package/MDAnalysis/coordinates/base.py	87.80%	5 Missing and 5 partials ⚠️

❗ There is a different number of reports uploaded between BASE (c4af00b) and HEAD (27597e7). Click for more details.

HEAD has 3 uploads less than BASE

Flag	BASE (c4af00b)	HEAD (27597e7)
	6	3

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4923      +/-   ##
===========================================
- Coverage    93.85%   85.84%   -8.01%     
===========================================
  Files          178      179       +1     
  Lines        22122    22293     +171     
  Branches      3129     3153      +24     
===========================================
- Hits         20763    19138    -1625     
- Misses         902     2702    +1800     
+ Partials       457      453       -4     

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

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[amruthesht]

Your thoughts on this are appreciated - @orbeckst

[orbeckst]

Thanks for the initial PR.

• The first big step is to get the tests running properly so that the CI uses an imdclient without IMDReader. Otherwise we are not sure we're testing the code here.
• Minor initial comments while I skimmed.
• Simple thing: run black over all files to get the formatting and ordering of imports right

[orbeckst]

I set the PR to Work in progress for the time being, just to indicate that we're not yet at the stage where the CI is working. Once the tests run properly, we can update the status.

Obviously, this shouldn't discourage anyone from contributing and commenting.

[IAlibay]

A first quick look

[jaclark5]

I read through and nothing jumped out at me that wasn't already mentioned, except that I didn't see these changes in the documentation that was linked. The following needs to be added:

Add doc/sphinx/source/documentation_pages/coordinates/IMD.rst
.. automodule:: MDAnalysis.coordinates.IMD

doc/sphinx/source/documentation_pages/coordinate_modules.rst
coordinates/IMD

doc/sphinx/source/documentation_pages/references.rst
If you use IMD capability...

[hmacdope]

Sorry for the delay, had a lookover, I will try and push some changes addressing some of these myself also, but would be good to pick up the momentum here again if possible.

[hmacdope]

@amruthesht do we have a plan for pushing ahead with suggested changes?

[amruthesht]

@amruthesht do we have a plan for pushing ahead with suggested changes?

Working on the imdclient repository right now. I had a few small issues there with using the IMDReader, but getting them fixed soon.
And then I'll wrap up some small changes on the mdanalysis end and we should be good to go!

[hmacdope]

@amruthesht kicking CI here.

[hmacdope]

@yuxuanzhuang would you be able to have a run through?

[yuxuanzhuang]

I will have another look at the PR this week. One immediate thing is to add documentation.

Please refer to the comments below.

I read through and nothing jumped out at me that wasn't already mentioned, except that I didn't see these changes in the documentation that was linked. The following needs to be added:

Add doc/sphinx/source/documentation_pages/coordinates/IMD.rst .. automodule:: MDAnalysis.coordinates.IMD

doc/sphinx/source/documentation_pages/coordinate_modules.rst coordinates/IMD

doc/sphinx/source/documentation_pages/references.rst If you use IMD capability...

[hmacdope]

Need to figure out tests also @amruthesht

[yuxuanzhuang]

I’ve added tests to cover most of the newly introduced code. I had to separate the test for the IMD version into its own file, as it modifies sys.modules and interferes with other tests. If someone can find a way to make it work without causing side effects, that would be preferable.

One other point worth discussing is the division of attributes between StreamReaderBase and IMDReader. I moved the initialization of self._frame to StreamReaderBase because some functions assume this attribute exists in the base class. Not sure about whether n_atoms should also be moved.

[hmacdope]

@orbeckst I have updated pins here for 0.2.2 and added the requisite minimum versions where appropriate. I think we might be almost good to go here.