fbergmann / libSEDML

SED-ML library based on libSBML
BSD 2-Clause "Simplified" License
9 stars 12 forks source link

'SetValue' no longer has child Parameters #86

Closed luciansmith closed 3 years ago

luciansmith commented 3 years ago

The 'SetValue' child of a RepeatedTask inherits from ComputeChange, which in turn inherits both from Change and from Calculation (in the spec).

(The 'Calculation' class in the spec is there just for the spec's convenience, and multiple inheritance was not intended to be included in the class structure of libSEDML.)

In libSEDML, the SedSetValue class inherits just from SedBase, so misses everything it is supposed to inherit.

(This is a change from previous behavior, but I'm not sure which version it was lost in.)

fbergmann commented 3 years ago

All attributes and values can be read, and written. So while the object structure of SedSetValue is quite a bit simpler, all attributes are present. Since you are using python and thus its duck-typing. You should be able to use it as is. Can you let me know where this fails for you?

luciansmith commented 3 years ago

Nope, I'm not using Python; I'm using C++. I used to be able to call 'getNumParameters', 'getParameter', 'createParameter'', and 'createVariable', and cannot any more.

fbergmann commented 3 years ago

sorry i assumed, since you asked for new python modules (which are ready). In any case ... it will take a bit longer to get the parameter / variable bits in there. So yes ... they would need to be added back in. They never made it into the version that was generated by deviser, and only were in the 0.4.4 branch of things.

fbergmann commented 3 years ago

@luciansmith do you need setvalue to inherit of computechange, or would you be ok with just having the list of variables / parameters back (as it was before)

luciansmith commented 3 years ago

No, it doesn't need to literally inherit from computechange. (Though presumably this would be a good idea at some point?) Just having the functions will be fine.

fbergmann commented 3 years ago

@luciansmith can you see whether this is working for you? I'll release when it is working for you.

luciansmith commented 3 years ago

It is indeed working! I was able to un-comment my old code again, and everything compiles. Thank you!

If you're updating things, would it be possible to also add 'name' to SedBase, cf https://github.com/fbergmann/libSEDML/issues/84 ?

fbergmann commented 3 years ago

i hope to get to 84 in the holidays. For now i thought it was urgent to fix the bug that had implications to simulations. Glad it worked out for you. I'll be sure to release a new version of the python bindings too.