Conversation
jakebeal
left a comment
jakebeal
left a comment
There was a problem hiding this comment.
Contents are good; style and testing needs improvement.
| @@ -1,3 +1,4 @@ | |||
| import sbol2.model | |||
There was a problem hiding this comment.
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.
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.
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.
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.
Too much whitespace here; please use pycodestyle to follow standard python conventions..
There was a problem hiding this comment.
This file isn't currently used. Please either incorporate into a test or delete.
There was a problem hiding this comment.
This file isn't currently used. Please either incorporate into a test or delete.
There was a problem hiding this comment.
This file isn't currently used. Please either incorporate into a test or delete.
There was a problem hiding this comment.
This file isn't currently used. Please either incorporate into a test or delete.
There was a problem hiding this comment.
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.
3 participants
Model conversion and test are done