-
Notifications
You must be signed in to change notification settings - Fork 1
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
How to add opil.Parameters with different namespace to a ProtocolInterface? #179
Comments
As far as I can tell from reading the SBOL spec, TopLevels and their children objects are required to have the same namespace. Can you please describe more about your other issue:
I'm not tracking how this follows from the issue about mixing namespaces. Thanks |
@bbartley Here is a screen capture of an opil parameter created before writing into xml. |
Sounds like a bug!
Can you confirm that the identity of the |
Hi @tramyn I am unable to reproduce the error you describe. Here was my attempt. Do you see some way to tweak this that will reproduce the error?.
|
@bbartley here is what I am doing to reproduce this bug: import opil
import sbol3
file_path = 'jellyfish_htc.xml'
opil_doc = opil.Document()
opil_doc.read(file_path, sbol3.RDF_XML)
# copy protocol_interface to avoid overwriting existing data
protocol_interfaces = [top_level.copy() for top_level in opil_doc.objects if isinstance(top_level, opil.ProtocolInterface)]
if len(protocol_interfaces) > 1:
raise Exception('Expecting 1 ProtocolInterface but %d were found' % len(protocol_interfaces))
# create new parameter to add to existing ProtocolInterface
protocol_interface = protocol_interfaces[0]
current_parameters = protocol_interface.has_parameter
new_parameters = [parameter for parameter in current_parameters]
string_parameter = opil.StringParameter('https://sd2e.org/ipd8aea1775c4041a1aa41c562319fca68')
string_parameter_value = opil.StringValue('https://sd2e.org/ip2c22022d655d4c8fadf7b605b684f338/StringValue1')
string_parameter_value.value = 'foo'
string_parameter_value.value_of = string_parameter
new_parameters.append(string_parameter)
# bug appears here when new_parameters are assigned to has_parameter
protocol_interface.has_parameter = new_parameters
for parameter in new_parameters:
print(parameter.identity) |
@tramyn I think you want to rearrange your code a little bit to make this work. You should add the There's no point setting identities on the So something like this (completely untested): string_parameter = opil.StringParameter()
# This sets string_parameter's identity
protocol_interface.has_parameter.append(string_parameter)
string_parameter_value = opil.StringValue()
string_parameter_value.value = 'foo'
# It is now save to add a reference to string_parameter because the identity has been set
string_parameter_value.value_of = string_parameter |
@tcmitchell Here is a little bit of background why adding parameters with different namespace made sense to me. Let's say there are 3 tools that need to communicate information about a opil.ProtocolInterface (ex: Aquarium -> Intent Parser -> Xplan -> Aquarium). Aquarium creates a opil.ProtocolInterface with initial parameters and default values. This information is then passed to Intent Parser to allow users who want to run this protocol to select/fill in their values. On the other end, Xplan requires that additional Parameters needs to be added to this ProtocolInterface in order to run an experiment. XPlan want users of Intent Parser to access and configure run information. Since these additional parameters wasn't originally created by Aquarium, it make sense that new parameters added to the ProtocolInterface should not take on the Aquarium namespace. Otherwise, this will imply that Aquarium wanted this level of information specified within it's ProtocolInterface. This is when Intent Parser's namespace is involved to include these additional parameters to an existing Aquarium ProtocolInterface. If opil requires that any Parameter added to a ProtocolInterface must take on the same namespace, then this information must be made clear in the opil specification. We did discuss in an opil call that these additional Parameters will be created as xplan's opil custom annotation so I will update Intent Parser base off of your suggestion. |
The requirement for naming of dependent objects comes from the SBOL3 specification, which underlies OPIL. Section 7.2 states:
pySBOL3 enforces this rule when the parent objects have SBOL-compliant URIs, which is why the identity you are passing into the constructor is overwritten when the object is added to a parent object. So it's not the fault of OPIL, it goes all the way back to the SBOL3 specification. |
@tcmitchell @bbartley The solution that I want to get here is to see how the opil library addresses this kind of problem and how will the library let a user know of this problem. Should opil silently allow users to create Parameter objects with different namespaces or should it throw an exception? Right now, opil silently allow users to specify multiple namespaces but then update namespaces for Parameters to conform to requirements mentioned in the spec. The
I think it is getting messy in terms of making users track information from two data language specification. It isn't easy separating which information applies to which specification so can the two libraries and its specification make it clearer for users like me to understand? |
It looks like pySBOL3 raises a |
We've had an offline conversation and figured out why the |
At a high level, it looks like @tramyn is copying a |
I want to add new parameters to an existing ProtocolInterface but when I do so, opil will modify these new parameters to use the ProtocolInterface namespace. Ideally, I want to distinguish where parameters are derived from so the new parameters uses Intent Parser's namespace and the ProtocolInterface points to a lab namespace (ex: http://aquarium.bio/htc/). However, opil will not allow mixtures of namespaces for parameters added to a ProtocolInterface and now I'm left with parameter values with the
valueOf
field not referenceable to its corresponding Parameter object.What is the best way to proceed here?
The text was updated successfully, but these errors were encountered: