sbmlteam / libsbml

LibSBML is a native library for reading, writing and manipulating files and data streams containing the Systems Biology Markup Language (SBML). It offers language bindings for C, C++, C#, Java, JavaScript, MATLAB, Perl, PHP, Python, R and Ruby.
https://sbml.org/software/libsbml
Other
39 stars 28 forks source link

Can't add ModelHistory without dates even in L3V2 #290

Closed avandecreme closed 1 year ago

avandecreme commented 1 year ago

265 adds support for ModelHistory without dates in L3V2.

However, when running the addModelHistory.c example with the parts adding the dates commented out on a L3V2 SBML file, we still get the following output: Set model history: invalid object.

The reason for that is that ModelHistory::hasRequiredAttributes is called on a model history which does not have a parent SBML object (yet) so we are falling in the level < 3 branch here: https://github.com/sbmlteam/libsbml/blob/226c8674bdfb5b28ac58b0b2046a44f2f8781bfd/src/sbml/annotation/ModelHistory.cpp#L382

fbergmann commented 1 year ago

is it possible to call ModelHistory::setParentSBMLObject before? Without knowing the sbml context, it is not possible to decide whether the element is valid or not. So i think the fix here would be to add extern C overloads for accessing the parent sbml object.

avandecreme commented 1 year ago

For the exact issue I guess we could fix it by setting (and unsetting it after the check) the parent before calling history->hasRequiredAttributes() here: https://github.com/sbmlteam/libsbml/blob/226c8674bdfb5b28ac58b0b2046a44f2f8781bfd/src/sbml/SBase.cpp#L2256

If someone wants to call ModelHistory::hasRequiredAttributes directly, I guess we have to make C bindings to access the parent sbml object indeed.

Another option would be to pass the SBML version as a parameter to hasRequiredAttributes but that would be quite a big change.

fbergmann commented 1 year ago

For the exact issue I guess we could fix it by setting (and unsetting it after the check) the parent before calling history->hasRequiredAttributes() here

i think that is could idea, if no parent has been set, we temporarily set before the check, and unset after.

avandecreme commented 1 year ago

Thanks for fixing!