Skip to content

Conversation

@Gonza10V
Copy link
Contributor

Closes #241
Closes #242

@Gonza10V Gonza10V requested a review from jakebeal October 22, 2025 21:09
@Gonza10V Gonza10V self-assigned this Oct 22, 2025
@Gonza10V
Copy link
Contributor Author

I would like to get feedback on this one. I'll try to get more of these implemented.

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.

Tests are failing.


# convert locations
locations2 = []
if seqfeat3.locations:
Copy link
Contributor

Choose a reason for hiding this comment

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

This can all be done with just a single list comprehension:
locations2 = [loc3.accept(self) for loc3 in seqfeat3.locations]

Copy link
Contributor Author

Choose a reason for hiding this comment

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

implemented

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also realized that I dont need to convert name, description, wasDerivedFrom, nor wasGeneratedBy as those are handled by _convert_identified. So I'm removing those.

elif type(loc3) is sbol3.location.EntireSequence:
raise NotImplementedError('Conversion of EntireSequence from SBOL3 to SBOL2 not yet implemented')
else: raise ValueError(f'Unknown location type {type(loc3)}, SequenceFeature cannot convert to SBOL2')
else: raise ValueError('SequenceFeature must have at least one location')
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to check this if we've validated in advance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed with the implementation of the list comprehension

# If they do have a component, their locations are added to the corresponding SBOL3 SubComponent.

# convert locations
locations3 = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar notes to above for converting to a list comprehension and dealing with exception handling. For pySBOL2 there's no accept function to call, but you can move the if/elif into a nested function and call that from the list comprehension.


# Create SBOL3 SequenceFeature
if seqanno2.component:
feature3 = sbol3.SubComponent(namespace=self._sbol3_namespace(seqanno2),
Copy link
Contributor

Choose a reason for hiding this comment

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

These aren't top-levels, so they don't take a namespace - they're going to get their name from their parent.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed

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.

SBOL 2-3 converter - Sequence_feature (Priority 1) SBOL 2-3 converter - Sequence_annotation (Priority 1)

3 participants