Conversation
…on(excluding assemblyplan MD)
…collections in sbol_doc instance var
…racted part objects in final document
|
closes #35 |
|
closes #36 |
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a9e3aac792
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| """ | ||
| backbone_def = extract_toplevel_definition(backbone_doc) | ||
| backbone_plasmid = MocloPlasmid(backbone_def.displayId, backbone_def, backbone_doc) | ||
| backbone_plasmid = Plasmid(backbone_def.displayId, backbone_def, backbone_doc) |
There was a problem hiding this comment.
translate_abstract_to_plasmids now instantiates Plasmid with backbone_def.displayId (a string) as the first argument, but Plasmid.__init__ expects a ComponentDefinition and immediately accesses definition.displayId; this raises AttributeError on every call, so abstract translation cannot run at all. The same constructor mismatch also appears in construct_plasmid_dict.
Useful? React with 👍 / 👎.
| plasmid_dict, antibiotic_resistance=KAN | ||
| ) | ||
| else: | ||
| compatible_plasmids = get_compatible_plasmids(plasmid_dict, self.backbone) |
There was a problem hiding this comment.
When a caller provides the optional backbone parameter, this branch still calls get_compatible_plasmids(..., self.backbone), but self.backbone is never defined in BuildCompiler; this path raises AttributeError and makes user-selected backbones unusable.
Useful? React with 👍 / 👎.
| if all( | ||
| s.identity != strain.identity | ||
| for s in existing.strain_definitions |
There was a problem hiding this comment.
This comprehension dereferences s.identity for every stored strain, but earlier indexing paths create Plasmid(..., strain_definition=None, ...), so existing.strain_definitions can contain None; in collections that include both plasmid implementations and strain module definitions, this throws AttributeError while indexing.
Useful? React with 👍 / 👎.
| ] # TODO update with more sophisticated selection process? | ||
| plasmid_cd = plasmid.plasmid_definition | ||
|
|
||
| transformation_activity = sbol2.Activity(f"transform_{chassis_md.name}") |
There was a problem hiding this comment.
The activity URI is derived only from chassis_md.name, so every loop iteration creates the same Activity identity for different plasmids; adding the second transformed plasmid to transformation_doc will hit URI collisions and abort instead of producing one transformation record per plasmid.
Useful? React with 👍 / 👎.
2 participants
Remaining TODO:
Optional TODO:
closes #33
closes #28
closes #35 #36