ChainedReader __repr__ fix for MemoryReader sub-readers

[ljwoods2]

Fixes #3349

Changes made in this Pull Request:

• If ChainedReader is initialized with filenames that are trajectory ndarrays rather than files, the stored filenames will be "None" for each ndarray rather than a filename
• The repr function will check if the filenames are ndarrays before calling os.path.basename

If there is a use case for a ChainedReader with both MemoryReader sub-readers and other sub-reader types, this solution won't work because it depends on all the filenames being the same type. Let me know and I can fix it!

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--4407.org.readthedocs.build/en/4407/

[github-actions]

Linter Bot Results:

Hi @ljwoods2! 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	⚠️ Possible failure

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/7849962018/job/21424417800

---

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

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (c4ffb7e) 93.38% compared to head (f5728a1) 93.38%.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #4407      +/-   ##
===========================================
- Coverage    93.38%   93.38%   -0.01%     
===========================================
  Files          171      185      +14     
  Lines        21737    22858    +1121     
  Branches      4012     4014       +2     
===========================================
+ Hits         20300    21346    +1046     
- Misses         951     1025      +74     
- Partials       486      487       +1     

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

[RMeli]

@ljwoods2 thank you for the PR. From a quick look, it looks like this PR contains un-related changes from #4402. Could you please fix this before we review? Thanks.

[pep8speaks]

Hello @ljwoods2! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2024-02-09 21:46:44 UTC

[ljwoods2]

@ljwoods2 thank you for the PR. From a quick look, it looks like this PR contains un-related changes from #4402. Could you please fix this before we review? Thanks.

Done, not sure how I managed to do that hahaha. Just reset the branch entirely

[RMeli]

Thanks @ljwoods2. I just had a look at the original issue #3349 and realised there is an open PR #3344 which includes a fix for the same issue. Unfortunately that PR seems to have stalled.

@mnmelo, would you be OK to address #3349 here, or do you prefer to keep it as part of #3344 (which we can hopefully get going again)?

[mnmelo]

Hi,

Definitely address it here. At least that much gets fixed, even if I don't get the time to advance #3344.

[lilyminium]

Thanks for making this much-needed fix, @ljwoods2! I left a couple comments -- IMO it'd be better to check all filenames than simply the first one.

The pep8 bot also has flagged a couple lines as not following PEP8 -- could you please fix those?

[ljwoods2]

@lilyminium Thank you for the feedback, I appreciate the pointers! Just revised based on your tips and changed repr to make more sense with the potential case of prepending an ndarray to a trajectory, let me know your thoughts.

[lilyminium]

Thanks for making these changes, they look really good @ljwoods2! This is a bit nitpicky but would you also mind adding a direct check of self.filenames (maybe as part of test_issue_3349)? You can access the universe's coordinate reader through u.trajectory[.filenames]. After that I reckon this would be 100% good to go!

[RMeli]

Nitpick, otherwise LGTM (besides @lilyminium outstanding requested change).

[ljwoods2]

@lilyminium Done, let me know what you think

[orbeckst]

@lilyminium could you please push this PR over the finish line as you have the only outstanding review and it seems to be almost done? Thanks!

[lilyminium]

LGTM -- thank you for the fix @ljwoods2!