Open Schmoho opened 2 years ago
Why is modelpolisher checking this at all? Should this not be done by the SBML validation? I.e. only check if the model is valid and what the validation warnings/errors are without any additional own checking.
In the end, the strict attribute is set on the model according to this inference. I guess the idea might be to allow to set strictness where possible - I assume the validator would not tell you if a model "could" be strict but isn't, right?
However I can't really say if this is actually a sensible thing to do, or to place within the scope of the Polisher. I.e. @draeger what's your take on this?
So if this is just checking if strict can be set I would do the following:
strict=False
strict=False
, set strict=True
and check if the model is still valid (i.e., does not have errors)
I.e. use a validation strategy instead of some heuristics.Exactly, ModelPolisher is not a model validator. It aims to set missing attributes to suitable values. It could, for instance, load a model that is invalid in the sence that the strict attribute isn't defined at all and then give it a value. The validation-based approach @matthiaskoenig suggests makes sense. The SBO term check is a nice addition. I assume ModelPolisher would also try to set those terms if undefined.
From the spec section 3.3 page 8 (line numbers at the end):
Instead, what we are checking right now is:
We are not explicitly checking lines 15 and 16, and the SBO-term check we do is not required by the spec.