ihmwg / python-ihm

Python package for handling IHM mmCIF and BinaryCIF files
MIT License
14 stars 7 forks source link

ihm.analysis.FilterStep.feature value is not checked #112

Closed aozalevsky closed 1 year ago

aozalevsky commented 1 year ago

ihm.analysis.FilterStep.feature that corresponds to the _ihm_modeling_post_process.feature which has controlled dictionary of values

https://github.com/ihmwg/python-ihm/blob/ccd52368ae0c1ab5e203aff53426d55ef0c2824c/ihm/analysis.py#L12

benmwebb commented 1 year ago

Yes, this is true. Usually we use a set of subclasses for enumerations like these but I think that's a bit heavyweight in this case. The simplest solution would be to use _text_choice_property from model.py. The minor complication is that you need to be careful when reading in a file that you don't fail to read a file that has an invalid value for this property.

aozalevsky commented 1 year ago

hm... should we have (or maybe we already have, but i missed it) multiple reader modes? like "strict", which follows the dictionary exactly and should fail in cases like this, and "relaxed" or similar that would parse anything non-standard value as a string?

benmwebb commented 1 year ago

There aren't many instances where a 'strict' mode would do anything useful, at least without a redesign, as python-ihm does not check against the dictionary itself during normal operation. So a 'strict' failure might mean that an input file wasn't compliant or it might mean that python-ihm hasn't been updated to match the latest version of the dictionary (e.g. a new crosslinker was added to the dictionary but not yet to the code). If you want to check a file is valid, better to run it through the validator.