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

A notebook that creates an example of an SBOL2 Generic Location #28

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

PrashantVaidyanathan
Copy link
Contributor

This addresses #26

@jakebeal
Copy link
Contributor

I think this isn't very informative yet, because it isn't concrete enough to really show the value of the representation.
How about instead having this show a Cre-Lox recombination applied to activate a not-yet-selected coding sequence by un-reversing it?

That would be a little more complicated, but I think a lot more compelling. The total representation would be:

  • Two Component objects, for the Cre-Lox sites
  • One SequenceAnnotation for the unknown CDS
  • One Range (first recombination site) and two GenericLocation (CDS and second site)
  • Two SequenceConstraint objects.

@PrashantVaidyanathan
Copy link
Contributor Author

@jakebeal - I agree with your comment there and I like the idea.
I have mixed feelings about including this example just for Generic Location because this has a lot more SBOL classes in it.

How about we have a simpler notebook that just shows Generic Location. But we can also add another notebook showing a Cre-Lox recombination? I've added another notebook (outside the CreatingSBOL2Objects folder).

Happy to hear your thoughts.

@PrashantVaidyanathan
Copy link
Contributor Author

We can also add a note pointing users to the Cre-Lox example (or perhaps a ReadMe in the sbol2 folder)

@jakebeal
Copy link
Contributor

I think there's no point in using GenericLocation without this level of complexity.

I would thus suggest that instead of making a low-content GenericLocation, that we have this notebook's documentation point to the notebooks for SequenceConstraint, SequenceAnnotation, etc. as prerequisites

…e pointing users to the more complex CreLox notebook.
@PrashantVaidyanathan
Copy link
Contributor Author

I've modified the Generic Location notebook to reflect your comments. I've also added a link to the Cre-Lox notebook while providing users with a simpler code snippet to understand how to create a Generic Location.

Copy link
Contributor

@jakebeal jakebeal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of my comments are "nice to haves", but I think it's important to avoid duplicating the loxP component and not add a sequence to the unknown CDS.

examples/sbol2/CreLoxRecombination.ipynb Outdated Show resolved Hide resolved
"\n",
"# Component for the second loxP site\n",
"loxP2 = sbol2.ComponentDefinition('loxP_site_2', sbol2.BIOPAX_DNA)\n",
"loxP2_seq = sbol2.Sequence('loxP_seq_2', 'ATAACTTCGTATAATGTATGCTATACGAAGTTAT', sbol2.SBOL_ENCODING_IUPAC)\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since these two sites have the same sequence, we should use only one ComponentDefinition for loxP, and include two different instances via Component objects.

"metadata": {},
"outputs": [],
"source": [
"import sbol2\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a giant block of code. I would suggest breaking it into chunks with key code comments moving to instead by notebook text.

"doc.addComponentDefinition(loxP2)\n",
"\n",
"# Create a Component for the CDS (Coding Sequence)\n",
"# The CDS is unknown or \"not yet selected,\" so we can use a generic placeholder sequence\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should not use any sequence at all. What you've got specified right here incorrectly constraints to an unknown of a specific length. Just omit the sequence.

examples/sbol2/CreLoxRecombination.ipynb Outdated Show resolved Hide resolved
examples/sbol2/CreLoxRecombination.ipynb Outdated Show resolved Hide resolved
examples/sbol2/CreLoxRecombination.ipynb Outdated Show resolved Hide resolved
"# Create SequenceConstraints to describe the relationships\n",
"# Constraint to ensure that loxP1 and CDS are adjacent\n",
"constraint1 = recombination_system.sequenceConstraints.create('loxP1_to_CDS')\n",
"constraint1.subject = loxP1_comp.identity\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe you need to pull identity out on any of these. I believe the assignment routine will do that automatically for you.

examples/sbol2/CreLoxRecombination.ipynb Outdated Show resolved Hide resolved
examples/sbol2/CreLoxRecombination.ipynb Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants