Multiple mutate fixes

[csbrasnett]

re-addresses #553 in the case that mutations are given across multiple chains. This PR makes sure that in the case that multiple chains are targeted, the check for the target residue existing is not present across all chains. So warnings aren't raised when the 'missing' residue is encountered.

[pckroon]

Thanks for picking up this issue.
I'm not very happy with how much code duplication this solution produces though...
Maybe you can abstract the duplicated functionality in a separate function (maybe that would return the affected nodes?)? As alternative, make the resspecs-to-find attributes of the AnnotateMutMod Processor, and make sure you find each at least once when running run_system.

def run_system(system):
    self.resspec_counts = ...
    # Modify run_molecule to increment the resspec_counter(s)
    super().run_system()
    if any(self.resspec_counts == 0): warn

[pckroon]

Nice bit of cleanup :) I do think I spotted a (small) mistake though, and there are some small comments left over from before.
I forgot why the different behaviour for whether a chain is specified or not?

[pckroon]

Updating the codecov action seems to have fixed the failing test thingie

[csbrasnett]

just to check, are you expecting any more changes from me?

[pckroon]

I think this still gives erroneous warnings for chemically disconnected things. For example, protein with 2 chains (A and B) which are not connected by a chemical bond (and are thus separate Molecules), combined with a valid resspec that defines a chain.

[csbrasnett]

I think this still gives erroneous warnings for chemically disconnected things. For example, protein with 2 chains (A and B) which are not connected by a chemical bond (and are thus separate Molecules), combined with a valid resspec that defines a chain.

If I've understood your concerns correctly, hopefully the two new tests satisfy them?

[pckroon]

One more testcase: I think {'resname': 'GLY', 'resid: 1'} will not print a warning even if it doesn't match anything.
After that I'm happy :)

[csbrasnett]

So, I think those tests do what you expect them to, but I'm not sure they should work, as they don't pick up on the error identification correctly? Eg, GLU4:GLY should mutate any GLU at resid 4 on every chain, but if resid 4 is not GLU then there should be a warning, and there currently isn't. I'll have a think about how to address this.

I'll also update the martinize2 help to add some more detail about what's possible with the mutate/modify flags?

[pckroon]

Eg, GLU4:GLY should mutate any GLU at resid 4 on every chain, but if resid 4 is not GLU then there should be a warning, and there currently isn't.

Exactly :)
I think you'll need to adapt the run_system method to track whether at the end there are any resspecs that didn't match anything

I'll also update the martinize2 help to add some more detail about what's possible with the mutate/modify flags?

This is always welcome. Documentation is not my strong suite

[csbrasnett]

Eg, GLU4:GLY should mutate any GLU at resid 4 on every chain, but if resid 4 is not GLU then there should be a warning, and there currently isn't.

Exactly :) I think you'll need to adapt the run_system method to track whether at the end there are any resspecs that didn't match anything

Isn't this already captured by the warning raised?

[pckroon]

That warning works for single molecules, not the entire system, which is what's going wrong if you have a resspec that does not mention any chains but does not match anywhere

[csbrasnett]

That warning works for single molecules, not the entire system, which is what's going wrong if you have a resspec that does not mention any chains but does not match anywhere

I'm pretty sure that possibility's covered by this test though:

[
    {'chain': 'A', 'resname': 'ALA', 'resid': 1},
    {'chain': 'A', 'resname': 'ALA', 'resid': 2},
    {'chain': 'A', 'resname': 'ALA', 'resid': 3},
    {'chain': 'B', 'resname': 'ALA', 'resid': 1},
    {'chain': 'B', 'resname': 'ALA', 'resid': 2},
    {'chain': 'B', 'resname': 'ALA', 'resid': 3}
],
[(0, 1), (1, 2), (3, 4), (4, 5)],
[({'resname': 'GLY', 'resid': 1}, 'ALA')],
True

ie. two molecules in the system, target any GLY with resid 1 (of which there are none), and expect a warning to be raised

[pckroon]

Since the test was always running on a single Molecule (rather than a System of multiple molecules) it was hiding the issue I was trying to dig up.
The problem is that if a resspec does not get applied to /all/ molecules, a warning gets issued, even if it applies to at least 1.

[csbrasnett]

thanks for that additional test, I see what you mean now about what wasn't captured. Will fix in due course!

[pckroon]

This test should raise a warning right? Since there is no GLY in chain A?

[csbrasnett]

you're right, I think I had confused something between that and test 8 sorry. I think now this should address the different possible cases, it was a bit of a headache getting something that satisfied both test 7 and test 8 together.

[pckroon]

No worries. I know how these things can happen.
My suggestion is to simplify the annotate_modifications function again, to simply apply yes or no depending on whether the resspec was matched anywhere. Being able to warn whether a modification applied to just one or all chains is nice to have, but not necessary. The most relevant message is when a modification doesn't apply anywhere. I'll leave it up to you whether the more detailed message is worth the added complexity.

[pckroon]

One last nitpick, LGTM otherwise!

[pckroon]

Nice work! Thanks for following through and dealing with all the small nitpicks!