-
Notifications
You must be signed in to change notification settings - Fork 29
Model conversion and test are done #314
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
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Contents are good; style and testing needs improvement.
@@ -1,3 +1,4 @@ | |||
import sbol2.model |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This import should not be needed, given the from sbol2
line below.
In fact, we should probably remove that one too since it makes the references in the code more ambiguous.
self.doc2.add(model2) | ||
# Map over all other TopLevel properties and extensions not covered by the constructor | ||
self._convert_toplevel(model3, model2) | ||
# seq2 = sbol2.Sequence(seq3.identity, seq3.elements, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this commented out code.
encoding2 = encoding_map.get(seq3.encoding, seq3.encoding) | ||
# Make the Sequence object and add it to the document | ||
seq2 = sbol2.Sequence(seq3.identity, seq3.elements, encoding=encoding2, version=self._sbol2_version(seq3)) | ||
seq2 = sbol2.Sequence(seq3.identity, seq3.elements, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't change the line-wrapping standard on the project. Per the setup.py file at the top, this project uses max-line-length = 119
.
There should not be any change to this line of code, since it's just a line-wrapping change.
def visit_model(self, model2: sbol2.model.Model): | ||
# Priority: 3 | ||
raise NotImplementedError('Conversion of Model from SBOL2 to SBOL3 not yet implemented') | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whitespace violates python conventions.
|
||
# Map over all other TopLevel properties and extensions not covered by the constructor | ||
self._convert_toplevel(model2, model3) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too much whitespace here; please use pycodestyle to follow standard python conventions..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file isn't currently used. Please either incorporate into a test or delete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file isn't currently used. Please either incorporate into a test or delete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file isn't currently used. Please either incorporate into a test or delete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file isn't currently used. Please either incorporate into a test or delete.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change this to sorted N-triples format, in order to allow direct file-to-file comparison around the loop.
Sorted N-triples guarantees consistent serialization, but TTL does not.
Model conversion and test are done