-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add cifdump CLI utility #283
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #283 +/- ##
=======================================
Coverage 88.98% 88.98%
=======================================
Files 40 40
Lines 2841 2841
=======================================
Hits 2528 2528
Misses 313 313
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great to me. The code is clean and follows the established template from rs.mtzdump
. I checked that the command line app works and the docs build and render correctly.
Thanks, @jrobsontull, for a most welcome addition to rs
!
Thanks for the welcome @kmdalton! I just corrected a final typo. Looking forward to contributing elsewhere. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me -- rs.cifdump
seems to function correctly as an analog to rs.mtzdump
. Thanks for the addition!
I triggered the CI workflows, and we can merge this in assuming all passes as expected |
Merged in -- thanks for the contribution, @jrobsontull! |
Given the recent change by the PDB to no longer provide MTZ files for structures (https://www.rcsb.org/news/feature/66ead31f2c751d5c77fbb706) it would be very useful to have a CLI tool equivalent to
mtzdump
for CIF files. We plan on using this repo at my workplace and this feature would be of great utility to us.This was brought up in #229 and the changes are relatively straightforward given that
rs
can already read CIF files. Most of the logic was taken from the existingmtzdump
utility.Testing
cifdump
can be accessed from the CLIcifdump
can be used to log the contents of a structure factor CIF file from the PDB