search

marrink-lab / vermouth-martinize

Describe and apply transformation on molecular structures and topologies
Apache License 2.0
86 stars 38 forks source link

Eunit flag Broken #459

Closed fgrunewald closed 1 year ago

fgrunewald commented 1 year ago

Someone messed up when implementing #422. The lines below are actually non-sense because they overwrite the e-unit criterion.

https://github.com/marrink-lab/vermouth-martinize/blob/aa53b3e3ca734032518bd56b3b8dad0831653612/bin/martinize2#L737-L741

fgrunewald commented 1 year ago

@pckroon I'd prefer we get rid of one of the flags either rb_region or rb_unit and for the reminaing flag we accepts either the keywords chain, molecule, all or a resid region

pckroon commented 1 year ago

That sounds like a reasonable solution. It makes the argparse part a bit more complicated due to argument validation, but not too much I think

fgrunewald commented 1 year ago

well the rb_region flag already takes both keywords and also resid lists at the moment

fgrunewald commented 1 year ago

@pckroon This is actually a bit more complicated than I thought: What should happen when we have one molecule consisting of two chains and define a resid region? Should the EN be formed between the chains or not?

pckroon commented 1 year ago

Logically all criteria should get ANDed. For now though, the CLI is limited to providing just a single criterion (right?). So yes, it should be formed between the chains.