Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The test if a plugin is loaded returns true when JSBML has a parser loaded #216

Open
draeger opened this issue Jun 2, 2020 · 6 comments
Labels

Comments

@draeger
Copy link
Member

draeger commented Jun 2, 2020

The test isSetPlugin on AbstractSBase should return true if the SBase has a child with the given namespace. In the current implementation, however, it returns true if JSBML has (at some point) loaded a parser for parsing that namespace.

For example, if a model has an FBC extension in package version 1, JSBML will return true when asking model.isSetPlugin with the namespace of FBC package version 2 as argument. For testing, the model 01190 from the SBML test suite can be used.

The desired behavior of this method (and the corresponding method hasExtension) is that it should only return true if an extension with the given namespace exists.

@hemilpanchiwala
Copy link

Along with this issue, while running the statements model.getPlugin(fbcNamespaceV1) == null and model.getPlugin(fbcNamespaceV2) == null for the model 01190 from the SBML test suite both returns false value. However, the case should be that one of them should return true value.

@niko-rodrigue
Copy link
Member

I can see that it can be confusing. We need at least to make the documentation clearer. My thinking on allowing namespace as well was to make it more convenient for people but make it work like when you use the package label, meaning returning the plugin if it does exist.
Also, if we make the method strict, it will force people to update their code when a new version of the package is out, even if the code would have work properly without change.

I don't think this method is use too much with namespace so we could change it's behaviour.

I see two options:

  • we let the method as it is and make documentation clearer about what happen when using namespaces. We could introduced hasExtensionStrict to have the strict version of the method if you think that would be useful.
  • we change the method to be strict and update the documentation to make it clear what happen when new versions of the package will be published.

What do you think ?

@niko-rodrigue
Copy link
Member

Also if you are really writing a piece of code for only a specific version of a package, you might just test it at the SBMLDocument level and stop right without doing anything else if it is not the right one.

@draeger
Copy link
Member Author

draeger commented Jun 5, 2020

@niko-rodrigue which convenient method in JSBML can be used to answer the question if an arbitrary instance of an SBase has any extension package (of a specific short label or namespace) attached?

@niko-rodrigue
Copy link
Member

niko-rodrigue commented Jun 5, 2020

@niko-rodrigue which convenient method in JSBML can be used to answer the question if an arbitrary instance of an SBase has any extension package (of a specific short label or namespace) attached?

Not really related to the issue but you have: https://sbmlteam.github.io/jsbml/files/doc/api/1.5/org/sbml/jsbml/SBase.html#getNumPlugins--

as well as SBase#getExtensionCount()

Or if I misunderstood your question, SBase#isSetPlugin

@draeger
Copy link
Member Author

draeger commented Jun 5, 2020

Ok, but these methods won't answer the question if a specific extension, such as fbc, is attached to an instance of SBase. So, we can check if there is any extension but then don't know which one, nor if the one of interest is there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants