Skip to content

[SPRINT-02-02] Split collection pulling from SBOL document indexing#101

Open
Gonza10V wants to merge 1 commit into
full_buildfrom
codex/refactor-collection-indexing-for-sbol-documents
Open

[SPRINT-02-02] Split collection pulling from SBOL document indexing#101
Gonza10V wants to merge 1 commit into
full_buildfrom
codex/refactor-collection-indexing-for-sbol-documents

Conversation

@Gonza10V
Copy link
Copy Markdown
Contributor

Motivation

  • Decouple SynBioHub network fetching from SBOL document indexing to make collection indexing reusable for local sbol2.Document objects and easier to test and debug.
  • Preserve existing constructor behavior that accepts SynBioHub URIs while avoiding hidden network pulls when indexing local documents.
  • Improve separation of concerns to make indexing helpers individually testable and to clarify failure modes (fetch vs indexing).

Description

  • Added pull_collection_uris(self, uris: list[str]) -> sbol2.Document to perform SynBioHub pull operations and raise a RuntimeError with URI context on fetch failure. (changed file: src/buildcompiler/buildcompiler.py).
  • Added index_sbol_document(self, doc: sbol2.Document, source: str = "local") to perform SBOL scanning/classification without invoking sbh.pull, and changed _index_collections into a compatibility wrapper that calls pull_collection_uris then index_sbol_document. (changed file: src/buildcompiler/buildcompiler.py).
  • Extracted smaller indexing helpers _index_implementation, _index_strain_module, _index_plasmid_or_backbone_definition, and _index_reagent_implementation and updated _index_document_objects signature to accept a source parameter; preserved existing list-based runtime state (indexed_plasmids, indexed_backbones, restriction_enzyme_implementations, ligase_implementations). (changed file: src/buildcompiler/buildcompiler.py).
  • Fixed an undefined-variable bug in _get_abstract_design by iterating self.sbol_doc.componentDefinitions.
  • Added targeted unit tests that exercise local-document indexing and the pull-then-index compatibility behavior. (changed file: tests/test_buildcompiler.py).

Testing

  • Ran pytest -k "index or collection or synbiohub" and observed all selected tests passed: 16 passed, 107 deselected.
  • Ran ruff check src/buildcompiler/buildcompiler.py tests/test_buildcompiler.py and the checks passed for the modified files.
  • Attempted the broader integration command uv run python -m unittest discover -s tests, which failed in this environment due to an external git dependency fetch error for SBOLInventory (HTTP 403), so full test discovery could not complete here.
  • Verified that local document indexing path does not call sbh.pull and that pull failures surface with URI context while indexing failures remain distinct via the new unit tests (tests/test_buildcompiler.py).

Codex Task

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 8cabb91feb

ℹ️ 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".

for strain in source_doc.moduleDefinitions:
self._index_strain_module(strain, implementation=None)

for definition in self.sbol_doc.componentDefinitions:
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Index component definitions from the input document

index_sbol_document accepts an arbitrary SBOL document, but _index_document_objects iterates self.sbol_doc.componentDefinitions instead of source_doc.componentDefinitions. In the new local-indexing flow (where the passed doc is not already merged into self.sbol_doc), plasmid/backbone definitions in that document are skipped, and related implementations can later fail object resolution. This makes local document indexing silently incomplete unless callers do an extra manual merge step.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant