Skip to content

Allow ChainReader to accept format keyword#2335

Merged
orbeckst merged 3 commits intoMDAnalysis:developfrom
lilyminium:format-chainreader
Sep 25, 2019
Merged

Allow ChainReader to accept format keyword#2335
orbeckst merged 3 commits intoMDAnalysis:developfrom
lilyminium:format-chainreader

Conversation

@lilyminium
Copy link
Member

Fixes #2334

Changes made in this Pull Request:

  • Passed format argument from ChainReader through to core._get_readers.get_reader_for

PR Checklist

@codecov
Copy link

codecov bot commented Aug 22, 2019

Codecov Report

Merging #2335 into develop will decrease coverage by 0.09%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           develop    #2335     +/-   ##
==========================================
- Coverage    89.89%   89.79%   -0.1%     
==========================================
  Files          173      173             
  Lines        21778    21786      +8     
  Branches      2861     2861             
==========================================
- Hits         19577    19563     -14     
- Misses        1609     1625     +16     
- Partials       592      598      +6
Impacted Files Coverage Δ
package/MDAnalysis/core/universe.py 94.98% <100%> (ø) ⬆️
package/MDAnalysis/coordinates/core.py 100% <100%> (ø) ⬆️
package/MDAnalysis/analysis/base.py 93.57% <0%> (-3.67%) ⬇️
package/MDAnalysis/coordinates/TRZ.py 81.74% <0%> (-1.53%) ⬇️
package/MDAnalysis/analysis/lineardensity.py 62.38% <0%> (-0.92%) ⬇️
package/MDAnalysis/lib/util.py 87.59% <0%> (-0.89%) ⬇️
package/MDAnalysis/analysis/gnm.py 95.38% <0%> (-0.77%) ⬇️
package/MDAnalysis/lib/formats/libdcd.pyx 78.31% <0%> (-0.65%) ⬇️
package/MDAnalysis/lib/transformations.py 78.18% <0%> (-0.44%) ⬇️
package/MDAnalysis/core/topologyobjects.py 98.02% <0%> (-0.4%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 57bc183...5db11b6. Read the comment docs.

Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comments.

Also, what happens if one provides a list of formats that parallels the list of files?

We actually have the functionality to provide tuples [(file, format), (file, format), ...] – how does your change interact with that feature?

@lilyminium
Copy link
Member Author

We actually have the functionality to provide tuples [(file, format), (file, format), ...] – how does your change interact with that feature?

Tuples override the format.
mda.Universe(PSF, [file, (file, 'pdb'), file, file, (file, 'gro')], format='xyz') results in readers = [XYZReader, PDBReader, XYZReader, XYZReader, GroReader]. This probably also merits a test. Incidentally, I didn't know about this feature until I was looking at the testsuite -- noting it down for documentation!

@orbeckst
Copy link
Member

Please just ping me when you need me to have a final look and 👍 .

@orbeckst
Copy link
Member

Incidentally, I didn't know about this feature until I was looking at the testsuite -- noting it down for documentation!

Well, the ChainReader is a bit of an oddity, but really quite useful and something that other packages can't do, as far as I know. It could do with good documentation.

@lilyminium
Copy link
Member Author

Please just ping me when you need me to have a final look and 👍 .

@orbeckst Added tests.

@orbeckst orbeckst merged commit e8df31e into MDAnalysis:develop Sep 25, 2019
@orbeckst
Copy link
Member

Thanks!

@lilyminium lilyminium deleted the format-chainreader branch April 14, 2020 08:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ChainReader does not respect format argument in Universe loading

2 participants