-
Notifications
You must be signed in to change notification settings - Fork 1
[ISSUE-04] Add BuildCompiler API skeleton #71
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
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| """Public BuildCompiler API skeleton.""" | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| from dataclasses import dataclass, field | ||
| from typing import Any | ||
|
|
||
| from .options import BuildOptions | ||
|
|
||
|
|
||
| @dataclass | ||
| class BuildCompiler: | ||
| """Lightweight dependency-injected compiler facade.""" | ||
|
|
||
| inventory: Any = None | ||
| sbol_document: Any = None | ||
| planner: Any = None | ||
| executor: Any = None | ||
| adapters: Any = None | ||
| options: BuildOptions = field(default_factory=BuildOptions) | ||
|
|
||
| @classmethod | ||
| def from_synbiohub( | ||
| cls, | ||
| *, | ||
| collections: list[str] | None = None, | ||
| sbh_registry: str | None = None, | ||
| auth_token: str | None = None, | ||
| sbol_doc: Any = None, | ||
| options: BuildOptions | None = None, | ||
| **kwargs: Any, | ||
| ) -> "BuildCompiler": | ||
| """Factory boundary reserved for future SynBioHub loading/indexing.""" | ||
| if collections: | ||
| raise NotImplementedError( | ||
| "Automatic SynBioHub collection loading/indexing is not implemented yet. " | ||
| "Inject inventory dependencies directly for now." | ||
| ) | ||
|
|
||
| return cls( | ||
| sbol_document=sbol_doc, | ||
| options=options or BuildOptions(), | ||
| **kwargs, | ||
| ) | ||
|
|
||
| def plan(self, abstract_designs: Any, options: BuildOptions | None = None) -> Any: | ||
| """Plan a build request via injected planner (placeholder in skeleton).""" | ||
| if self.planner is None: | ||
| raise NotImplementedError( | ||
| "Build planning is not implemented in the API skeleton. " | ||
| "Inject a planner dependency to use BuildCompiler.plan()." | ||
| ) | ||
|
|
||
| effective_options = options or self.options | ||
| return self.planner.plan(abstract_designs, options=effective_options) | ||
|
|
||
| def execute(self, plan: Any, options: BuildOptions | None = None) -> Any: | ||
| """Execute a build plan via injected executor (placeholder in skeleton).""" | ||
| if self.executor is None: | ||
| raise NotImplementedError( | ||
| "Build execution is not implemented in the API skeleton. " | ||
| "Inject an executor dependency to use BuildCompiler.execute()." | ||
| ) | ||
|
|
||
| effective_options = options or self.options | ||
| return self.executor.execute(plan, options=effective_options) | ||
|
|
||
| def full_build(self, abstract_designs: Any, options: BuildOptions | None = None) -> Any: | ||
| """Convenience skeleton method: plan then execute.""" | ||
| plan = self.plan(abstract_designs, options=options) | ||
| return self.execute(plan, options=options) | ||
|
|
||
|
|
||
| def full_build( | ||
| abstract_designs: Any, | ||
| *, | ||
| inventory: Any = None, | ||
| sbol_document: Any = None, | ||
| planner: Any = None, | ||
| executor: Any = None, | ||
| adapters: Any = None, | ||
| options: BuildOptions | None = None, | ||
| collections: list[str] | None = None, | ||
| sbh_registry: str | None = None, | ||
| auth_token: str | None = None, | ||
| sbol_doc: Any = None, | ||
| **kwargs: Any, | ||
| ) -> Any: | ||
| """Module-level full-build entry point for the public API skeleton.""" | ||
| compiler_options = options or BuildOptions() | ||
|
|
||
| if collections is not None or sbh_registry is not None or auth_token is not None or sbol_doc is not None: | ||
| compiler = BuildCompiler.from_synbiohub( | ||
| collections=collections, | ||
| sbh_registry=sbh_registry, | ||
| auth_token=auth_token, | ||
| sbol_doc=sbol_doc, | ||
| options=compiler_options, | ||
| inventory=inventory, | ||
| planner=planner, | ||
| executor=executor, | ||
| adapters=adapters, | ||
| **kwargs, | ||
| ) | ||
| else: | ||
| compiler = BuildCompiler( | ||
| inventory=inventory, | ||
| planner=planner, | ||
| executor=executor, | ||
| adapters=adapters, | ||
| options=compiler_options, | ||
| **kwargs, | ||
| ) | ||
|
|
||
| return compiler.full_build(abstract_designs, options=compiler_options) | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,132 @@ | ||
| import sys | ||
|
|
||
| import pytest | ||
|
|
||
| from buildcompiler.api import BuildCompiler, BuildOptions, full_build | ||
|
|
||
|
|
||
| class FakePlanner: | ||
| def __init__(self): | ||
| self.calls = [] | ||
|
|
||
| def plan(self, abstract_designs, *, options): | ||
| self.calls.append((abstract_designs, options)) | ||
| return {"plan": abstract_designs, "options": options} | ||
|
|
||
|
|
||
| class FakeExecutor: | ||
| def __init__(self): | ||
| self.calls = [] | ||
|
|
||
| def execute(self, plan, *, options): | ||
| self.calls.append((plan, options)) | ||
| return {"result": plan, "options": options} | ||
|
|
||
|
|
||
| def test_import_smoke(): | ||
| assert BuildCompiler is not None | ||
| assert full_build is not None | ||
| assert BuildOptions is not None | ||
|
|
||
|
|
||
| def test_constructor_defaults_and_injected_dependencies(): | ||
| compiler = BuildCompiler() | ||
| assert compiler.inventory is None | ||
| assert compiler.sbol_document is None | ||
| assert compiler.planner is None | ||
| assert compiler.executor is None | ||
| assert compiler.adapters is None | ||
| assert isinstance(compiler.options, BuildOptions) | ||
|
|
||
| inventory = object() | ||
| sbol_document = object() | ||
| planner = object() | ||
| executor = object() | ||
| adapters = object() | ||
|
|
||
| injected = BuildCompiler( | ||
| inventory=inventory, | ||
| sbol_document=sbol_document, | ||
| planner=planner, | ||
| executor=executor, | ||
| adapters=adapters, | ||
| ) | ||
|
|
||
| assert injected.inventory is inventory | ||
| assert injected.sbol_document is sbol_document | ||
| assert injected.planner is planner | ||
| assert injected.executor is executor | ||
| assert injected.adapters is adapters | ||
|
|
||
|
|
||
| def test_api_import_does_not_load_optional_automation_modules(): | ||
| assert "pudupy" not in sys.modules | ||
| assert "opentrons" not in sys.modules | ||
| assert "SBOLInventory" not in sys.modules | ||
|
|
||
|
|
||
| def test_from_synbiohub_placeholder_without_collection_loading(): | ||
| compiler = BuildCompiler.from_synbiohub( | ||
| collections=[], | ||
| sbh_registry=None, | ||
| auth_token=None, | ||
| sbol_doc=None, | ||
| ) | ||
| assert isinstance(compiler, BuildCompiler) | ||
|
|
||
|
|
||
| def test_from_synbiohub_raises_when_collection_loading_is_requested(): | ||
| with pytest.raises(NotImplementedError, match="collection loading/indexing"): | ||
| BuildCompiler.from_synbiohub(collections=["https://example.org/collection"]) | ||
|
|
||
|
|
||
| def test_plan_execute_raise_without_injected_dependencies(): | ||
| compiler = BuildCompiler() | ||
| with pytest.raises(NotImplementedError, match="planning"): | ||
| compiler.plan({"x": 1}) | ||
|
|
||
| with pytest.raises(NotImplementedError, match="execution"): | ||
| compiler.execute({"plan": 1}) | ||
|
|
||
|
|
||
| def test_plan_execute_full_build_delegate_to_injected_dependencies(): | ||
| planner = FakePlanner() | ||
| executor = FakeExecutor() | ||
| options = BuildOptions() | ||
| compiler = BuildCompiler(planner=planner, executor=executor, options=options) | ||
|
|
||
| plan = compiler.plan("design") | ||
| result = compiler.execute(plan) | ||
| full = compiler.full_build("design") | ||
|
|
||
| assert plan["plan"] == "design" | ||
| assert result["result"]["plan"] == "design" | ||
| assert full["result"]["plan"] == "design" | ||
| assert planner.calls | ||
| assert executor.calls | ||
|
|
||
|
|
||
| def test_module_level_full_build_uses_lightweight_constructor_path(): | ||
| planner = FakePlanner() | ||
| executor = FakeExecutor() | ||
|
|
||
| result = full_build("design", planner=planner, executor=executor) | ||
|
|
||
| assert result["result"]["plan"] == "design" | ||
|
|
||
|
|
||
| def test_module_level_full_build_uses_synbiohub_factory_path_when_requested(): | ||
| planner = FakePlanner() | ||
| executor = FakeExecutor() | ||
|
|
||
| result = full_build( | ||
| "design", | ||
| planner=planner, | ||
| executor=executor, | ||
| collections=[], | ||
| sbh_registry=None, | ||
| auth_token=None, | ||
| sbol_doc=None, | ||
| ) | ||
|
|
||
| assert result["result"]["plan"] == "design" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
sbol_documentin module-levelfull_buildThe module-level
full_build(...)accepts ansbol_documentargument, but in the lightweight constructor path it is never passed toBuildCompiler, so callers who providesbol_document(without SynBioHub args) silently lose that input and run withcompiler.sbol_document=None. This breaks dependency-injected flows where planner/executor expect the document to be present and makes the public API contract misleading.Useful? React with 👍 / 👎.
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.
@codex good catch, solve this issue
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.
Summary
full_build(...)lightweight path to correctly forwardsbol_documentintoBuildCompiler(...), so callers don’t lose that injected input when SynBioHub args are not used.sbol_documentis preserved in the lightweight path by monkeypatchingBuildCompiler.full_buildand asserting the captured instance value matches the provided document.Fix full_build forwarding of sbol_document.Testing
pytest tests/unit/api/test_compiler_api.pyView task →
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.
@codex good catch, solve this issue
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.
Summary
full_build(...)lightweight path to forwardsbol_documentintoBuildCompiler(...), preventing silent loss of caller-provided SBOL input when SynBioHub args are not used. src/buildcompiler/api/compiler.pyL105-L116BuildCompiler.full_build, invokes module-levelfull_build(...)withsbol_document, and asserts the constructed compiler preserves that exact object. tests/unit/api/test_compiler_api.pyL118-L132Fix full_build forwarding of sbol_document(commit5295347).Testing
pytest tests/unit/api/test_compiler_api.pyView task →