SED-ML / sed-ml

Simulation Experiment Description Markup Language (SED-ML)
http://sed-ml.org
5 stars 2 forks source link

Make `model.language` attribute required instead of optional #147

Closed matthiaskoenig closed 3 years ago

matthiaskoenig commented 3 years ago

Currently the model.language field is optional. The language of the model is essential for a tool to decide if they can support the given model and tasks using the model. In my opinion this field should be required. Otherwise every tool must implement methods to figure out what the model language could be (and in this case there is no need for a language attribute because code exists to figure out the language).

I cannot see any reason why the language attribute should be optional, besides putting additional work on tool developers.

luciansmith commented 3 years ago

This seems reasonable to me, but do remember that for what it's worth, nothing validates optional vs. required attributes yet (that I know of, at least.)

matthiaskoenig commented 3 years ago

We even strongly advocate this in the spec

However, the use of the language attribute is strongly encouraged for two reasons. Firstly, it helps to decide whether or not one is able to run the simulation, that is to parse the model referenced in the SED-ML file. Secondly, the language attribute is also needed to decide how to handle the Symbols in the Variable class, as the interpretation of Symbols depends on the language of the representation format.

Especially with supporting more model languages we should make this required.

luciansmith commented 3 years ago

This change is now incorporated into the draft spec, BUT it still needs to be voted on. Assuming Matthias still wants the change, this means that at least two of Frank, Tomas, Jonathan, or David should agree to the change (or officially abstain); please reply to this if you agree with the change!

fbergmann commented 3 years ago

I'm happy with the change to make it a required attribute.

matthiaskoenig commented 3 years ago

I still want this ;)

nickerso commented 3 years ago

I very much agree with this change.

luciansmith commented 3 years ago

That's three of you! I'm counting it as approved and am closing the issue.