douweschulte / pdbtbx

A library to open/edit/save (crystallographic) Protein Data Bank (PDB) and mmCIF files in Rust.
https://crates.io/crates/pdbtbx
MIT License
54 stars 17 forks source link

Downgrade remark type number invalid error #124

Closed y1zhou closed 6 months ago

y1zhou commented 6 months ago

Fixes #123 and adds a test PDB file containing such REMARK records.

y1zhou commented 6 months ago

@douweschulte It seems the ReadOptions hasn't made its way into the core parser yet? The open_with_options function only passes ReadOptions.level into the open_pdb / open_mmcif function and ignores all other fields.

https://github.com/douweschulte/pdbtbx/blob/7af6cc8448756e009e08651ca701dc8cb79c6c17/src/read/general.rs#L34-L50

To implement the only_atomic_data read option we'd need corresponding open_pdb_with_options and open_mmcif_with_options as well, right? I assume you don't want to modify the original signatures directly for backward compatibility.

douweschulte commented 6 months ago

Yes something like that has to be added. On not changing the API, you can change the 'core' methods (meaning the one that actually does the parsing), but make sure to keep this one private and keep the current open functions as calls to the core functions with the default ReadOptions for example.

OWissett commented 6 months ago

I have created another issue for finishing my implementation of the ReadOptions. #125

Sorry for leaving it half done, I got stuck with some other PhD work.

y1zhou commented 6 months ago

Cool I'll try to add in the functions that parse ReadOptions.

Sorry for leaving it half done, I got stuck with some other PhD work.

No worries we are all aware how stressful that can be :)

y1zhou commented 6 months ago

Perfect!! I am fine with merging right away and doing the only_load_atomic together with #125 in another PR or doing that work right here whatever works best for you.

Yes taking that in another PR seems more reasonable. Will send as more work is done!