Implement wave container commands#3954
Conversation
JulianFlesch
changed the title
| ] | ||
| return containers_flat | ||
|
|
||
| def _resolve_module_dir(self, module: str | Path) -> Path: |
There was a problem hiding this comment.
do we not have this funciton already somehwere else?
There was a problem hiding this comment.
I might be overlooking something, but not really ... As far as I can tell, we have ModulesInfo, which was used at some point, but it never provides the environment.yml or meta.yml paths, or confirms their existance.
Maybe this should be in ModulesInfo?
| return module_dir | ||
|
|
||
| @staticmethod | ||
| def _environment_path(module_dir: Path) -> Path: |
There was a problem hiding this comment.
because we lint for this, I think we should be able to reuse something there.
|
|
||
|
|
||
| @contextmanager | ||
| def set_wd_tempdir() -> Generator[None, None, None]: |
There was a problem hiding this comment.
this function is for sure created somewhere else already.
There was a problem hiding this comment.
Yes, In pipeline downloads! But that version throws Download-specific errors ...
The plan was move it out of the utils.py over there into the general utils.
But now we don't use it at all anymore ...
Should we remove it again?
|
|
Added the meta_yml sorting now. I also switched the default value of |
mashehu
force-pushed
the
feature/3952-create-waveyml
branch
from
1de5380 to
b92491a
Compare
|
|
||
| # Load containers from meta.yml into the component before linting | ||
| meta_path = Path(self.nfcore_component.component_dir, "meta.yml") | ||
| if meta_path.exists(): |
There was a problem hiding this comment.
no need for this if clause, we already check that in read_meta_yml()
mashehu
force-pushed
the
feature/3952-create-waveyml
branch
3 times, most recently
from
47ed327 to
b71b250
Compare
| return formatted_topics | ||
|
|
||
|
|
||
| def obtain_containers(_, containers: dict) -> dict: |
There was a problem hiding this comment.
replace with get_containers_from_meta or pydantic model
| name_hash = tag.rsplit("--", 1)[1] | ||
| else: | ||
| tag_lower = tag.lower() | ||
| if len(tag_lower) >= 8 and all(c in "0123456789abcdef" for c in tag_lower): |
There was a problem hiding this comment.
isn't there a better way to check for hexadecimal characters?
| pass | ||
|
|
||
|
|
||
| def get_container_with_regex(main_nf_path: Path, component_name: str | None = None) -> str: |
There was a problem hiding this comment.
add test for this function
| from ..test_modules import TestModules | ||
|
|
||
|
|
||
| class TestModuleContainers: |
There was a problem hiding this comment.
| class TestModuleContainers: | |
| class TestModuleContainers(TestModules): |
I think
| def _write_meta(self, module_dir: Path, meta: dict) -> None: | ||
| (module_dir / "meta.yml").write_text(yaml.safe_dump(meta), encoding="utf-8") | ||
|
|
||
| def _containers_by_system(self, prefix: str = "testC") -> dict: |
There was a problem hiding this comment.
replace with pydantic model
| assert manager.module_directory == module_dir | ||
| assert manager.environment_yml == module_dir / "environment.yml" | ||
| assert manager.meta_yml == module_dir / "meta.yml" | ||
|
|
There was a problem hiding this comment.
add one integration test to check that the wave logic actually works and doesn't change in the background. maybe with --dry-run
| with mock.patch.object(manager, "get_containers_from_meta", return_value=containers): | ||
| listed = manager.list_containers() |
There was a problem hiding this comment.
don't mock this, run this against the gitlab test repo with one specific module
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
| self.is_patched: bool = False | ||
| self.branch: str | None = None | ||
| self.workflow_name: str | None = None | ||
| self.container: dict = {} |
There was a problem hiding this comment.
| self.container: dict = {} |
Never set or used here. If it is initialized later then this would be bad design.
There was a problem hiding this comment.
Turns out we need this for mypy https://github.com/nf-core/tools/actions/runs/26280949574/job/77356703978?pr=3954#step:3:48
will revert this commit
Co-authored-by: JulianFlesch <julianflesch@googlemail.com>
Co-authored-by: JulianFlesch <julianflesch@googlemail.com>
This reverts commit 3c32ee0.
Co-authored-by: Matthias Hörtenhuber <mashehu@users.noreply.github.com>
3 participants
Closes #3952