bienchen commented 2 years ago

For ModelArchive we need the _struct.pdbx_model_details field. I tried to add it to ihm by extending ihm.System and the struct dumper. Unit tests were also adapted by creating file output and running diff to make sure that just the _struct.pdbx_model_details line was added. Documentation builds and shows the changes.

codecov-commenter commented 2 years ago

Codecov Report

Merging #79 (fcb91fc) into main (7dd27da) will increase coverage by 0.00%. The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main      #79   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          28       28           
  Lines        7014     7015    +1     
  Branches     1663     1663           
=======================================
+ Hits         6998     6999    +1     
  Misses         13       13           
  Partials        3        3

Impacted Files	Coverage Δ
ihm/dumper.py	`100.00% <ø> (ø)`
ihm/__init__.py	`99.78% <100.00%> (+<0.01%)`	:arrow_up:

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 7dd27da...fcb91fc. Read the comment docs.

benmwebb commented 2 years ago

This needs to be rebased to apply cleanly. (I can certainly do this, but would rather you did!) I recommend you have a branch in your repo which tracks the main branch of ihmwg/python-ihm. Call it 'upstream' if you like. You can sync this with upstream from the GitHub website (or via git commands if you prefer). Then make a new branch for each PR (e.g. git checkout -b my-new-pr). This will reduce the risk of rebase conflicts, and the PRs will look a lot cleaner (e.g. this PR shows 7 commits, but really only one of them is related to this change). And your updates of the upstream branch will then be simple fast-forwards, not awkward merges.

Also, if you add a new field to a python-ihm object and output it with dumper.py, you need to add a corresponding handler in reader.py to read it back in. Something like the following should work:

class _StructHandler(Handler):
    category = '_struct'

    def __call__(self, title, entry_id, pdbx_model_details):
        self.copy_if_present(self.system, locals(), keys=('title',),
                             mapkeys={'entry_id': 'id',
                                      'pdbx_model_details': 'model_details'})

Then you'll need a test case in test_reader.py which, given a chunk of mmCIF, tests that the resulting Python objects have the right data. In this case, a simple modification to the test_struct_handler method should be all that's needed.

Let me know if anything wasn't clear, and whether you want to update this PR accordingly. Alternatively, I can take what you have so far and rebase it and add the reader support.

ihmwg / python-ihm

Add _struct.pdbx_model_details #79

Codecov Report