Skip to content
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

Modules restructure #1859

Merged
merged 29 commits into from
Oct 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
6cfe78e
change path for testing module structure
mirpedrol Sep 26, 2022
ae958d1
change module folder structure from pipeline template
mirpedrol Sep 26, 2022
1c8e634
change path from pipeline template
mirpedrol Sep 26, 2022
11e5d0d
change path from module_utils
mirpedrol Sep 26, 2022
ac38a14
change paths added to pytest_modules.yml
mirpedrol Sep 26, 2022
5235405
modify modules create command
mirpedrol Sep 26, 2022
05f1cb4
fix create-test-yml
mirpedrol Sep 26, 2022
7a2d46e
add variables to ModuleCommand
mirpedrol Sep 26, 2022
e2ec9a7
fix paths for linting modules
mirpedrol Sep 27, 2022
a7d62a3
fix paths for bump-versions
mirpedrol Sep 27, 2022
3cd1336
try to add any custom folder as nf-core inside modules
mirpedrol Sep 27, 2022
139818f
modules_json.py working (hopefully)
mirpedrol Sep 27, 2022
2d40c7e
lint bug fixed but lint not passing :(
mirpedrol Sep 27, 2022
cb7e95a
pipeline linting passing
mirpedrol Sep 30, 2022
2433fe8
modify modules structure with two levels modules/nf-core or modules/c…
mirpedrol Sep 30, 2022
4cc55e5
fix some pytests
mirpedrol Oct 1, 2022
8366a77
more tests fixed
mirpedrol Oct 2, 2022
f7bdf41
fix sha tests
mirpedrol Oct 3, 2022
58f1491
fix gitlab workflow
mirpedrol Oct 3, 2022
94231c4
fix linting
mirpedrol Oct 3, 2022
3a02093
gitlab force
mirpedrol Oct 3, 2022
e96386e
fix yaml sha
mirpedrol Oct 3, 2022
dd7fb4f
fix isort
mirpedrol Oct 3, 2022
aa0e6c8
Merge branch 'dev' into modules-restructure
mirpedrol Oct 3, 2022
79a48fd
remove branch and url from specific modules command
mirpedrol Oct 4, 2022
9ddc614
update changelog
mirpedrol Oct 4, 2022
683568a
change nf-core/modules branch to master
mirpedrol Oct 4, 2022
cb0ab31
change sha for master branch
mirpedrol Oct 4, 2022
ee8459a
update custom/dumpsoftwareversions to last commit
mirpedrol Oct 4, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/create-lint-wf.yml
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ jobs:
run: nf-core --log-file log.txt modules install fastqc --dir nf-core-testpipeline/ --force

- name: nf-core modules install gitlab
run: nf-core --log-file log.txt modules --git-remote https://gitlab.com/nf-core/modules-test.git install fastqc --dir nf-core-testpipeline/
run: nf-core --log-file log.txt modules --git-remote https://gitlab.com/nf-core/modules-test.git --branch main-restructure install fastqc --force --dir nf-core-testpipeline/

- name: nf-core modules list local
run: nf-core --log-file log.txt modules list local --dir nf-core-testpipeline/
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
- Use contextlib to temporarily change working directories ([#1819](https://github.com/nf-core/tools/pull/1819))
- More helpful error messages if `nf-core download` can't parse a singularity image download
- Modules: If something is wrong with the local repo cache, offer to delete it and try again ([#1850](https://github.com/nf-core/tools/issues/1850))
- Restructure code to work with the directory restructuring in [modules](https://github.com/nf-core/modules/pull/2141) ([#1859](https://github.com/nf-core/tools/pull/1859))

### Modules

Expand Down
8 changes: 7 additions & 1 deletion nf_core/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -661,7 +661,13 @@ def create_test_yml(ctx, tool, run_tests, output, force, no_prompts):
the required `test.yml` file based on the output files.
"""
try:
meta_builder = nf_core.modules.ModulesTestYmlBuilder(tool, run_tests, output, force, no_prompts)
meta_builder = nf_core.modules.ModulesTestYmlBuilder(
module_name=tool,
run_tests=run_tests,
test_yml_output_path=output,
force_overwrite=force,
no_prompts=no_prompts,
)
meta_builder.run()
except (UserWarning, LookupError) as e:
log.critical(e)
Expand Down
27 changes: 14 additions & 13 deletions nf_core/lint/modules_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,24 +28,25 @@ def modules_json(self):

for repo in modules_json_dict["repos"].keys():
# Check if the modules.json has been updated to keep the
if "modules" not in modules_json_dict["repos"][repo] or "git_url" not in modules_json_dict["repos"][repo]:
if "modules" not in modules_json_dict["repos"][repo] or not repo.startswith("http"):
failed.append(
"Your `modules.json` file is outdated. "
"Please remove it and reinstall it by running any module command."
"It will be automatically generated by running any module command."
)
continue

for module, module_entry in modules_json_dict["repos"][repo]["modules"].items():
if not Path(modules_dir, repo, module).exists():
failed.append(
f"Entry for `{Path(repo, module)}` found in `modules.json` but module is not installed in "
"pipeline."
)
all_modules_passed = False
if module_entry.get("branch") is None:
failed.append(f"Entry for `{Path(repo, module)}` is missing branch information.")
if module_entry.get("git_sha") is None:
failed.append(f"Entry for `{Path(repo, module)}` is missing version information.")
for dir in modules_json_dict["repos"][repo]["modules"].keys():
for module, module_entry in modules_json_dict["repos"][repo]["modules"][dir].items():
if not Path(modules_dir, dir, module).exists():
failed.append(
f"Entry for `{Path(modules_dir, dir, module)}` found in `modules.json` but module is not installed in "
"pipeline."
)
all_modules_passed = False
if module_entry.get("branch") is None:
failed.append(f"Entry for `{Path(modules_dir, dir, module)}` is missing branch information.")
if module_entry.get("git_sha") is None:
failed.append(f"Entry for `{Path(modules_dir, dir, module)}` is missing version information.")
if all_modules_passed:
passed.append("Only installed modules found in `modules.json`")
else:
Expand Down
2 changes: 1 addition & 1 deletion nf_core/module-template/tests/main.nf
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

nextflow.enable.dsl = 2

include { {{ tool_name_underscore|upper }} } from '../../../{{ "../" if subtool else "" }}modules/{{ tool_dir }}/main.nf'
include { {{ tool_name_underscore|upper }} } from '../../../../{{ "../" if subtool else "" }}modules/nf-core/{{ tool_dir }}/main.nf'

workflow test_{{ tool_name_underscore }} {
{% if has_meta %}
Expand Down
1 change: 1 addition & 0 deletions nf_core/modules/bump_versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import logging
import re
from pathlib import Path

import questionary
import rich
Expand Down
27 changes: 15 additions & 12 deletions nf_core/modules/create.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,12 @@
import nf_core.modules.module_utils
import nf_core.utils

from .modules_command import ModuleCommand

log = logging.getLogger(__name__)


class ModuleCreate(object):
class ModuleCreate(ModuleCommand):
def __init__(
self,
directory=".",
Expand All @@ -38,6 +40,7 @@ def __init__(
conda_version=None,
repo_type=None,
):
super().__init__(directory)
self.directory = directory
self.tool = tool
self.author = author
Expand Down Expand Up @@ -72,10 +75,10 @@ def create(self):

If <directory> is a clone of nf-core/modules, it creates or modifies the following files:

modules/modules/tool/subtool/
modules/modules/nf-core/tool/subtool/
* main.nf
* meta.yml
modules/tests/modules/tool/subtool/
modules/tests/modules/nf-core/tool/subtool/
* main.nf
* test.yml
* nextflow.config
Expand Down Expand Up @@ -250,13 +253,13 @@ def create(self):
pytest_modules_yml = yaml.safe_load(fh)
if self.subtool:
pytest_modules_yml[self.tool_name] = [
f"modules/{self.tool}/{self.subtool}/**",
f"tests/modules/{self.tool}/{self.subtool}/**",
f"modules/nf-core/{self.tool}/{self.subtool}/**",
f"tests/modules/nf-core/{self.tool}/{self.subtool}/**",
]
else:
pytest_modules_yml[self.tool_name] = [
f"modules/{self.tool}/**",
f"tests/modules/{self.tool}/**",
f"modules/nf-core/{self.tool}/**",
f"tests/modules/nf-core/{self.tool}/**",
]
pytest_modules_yml = dict(sorted(pytest_modules_yml.items()))
with open(os.path.join(self.directory, "tests", "config", "pytest_modules.yml"), "w") as fh:
Expand Down Expand Up @@ -323,8 +326,8 @@ def get_module_dirs(self):
file_paths[os.path.join("modules", "main.nf")] = module_file

if self.repo_type == "modules":
software_dir = os.path.join(self.directory, "modules", self.tool_dir)
test_dir = os.path.join(self.directory, "tests", "modules", self.tool_dir)
software_dir = os.path.join(self.directory, self.default_modules_path, self.tool_dir)
test_dir = os.path.join(self.directory, self.default_tests_path, self.tool_dir)

# Check if module directories exist already
if os.path.exists(software_dir) and not self.force_overwrite:
Expand All @@ -334,8 +337,8 @@ def get_module_dirs(self):
raise UserWarning(f"Module test directory exists: '{test_dir}'. Use '--force' to overwrite")

# If a subtool, check if there is a module called the base tool name already
parent_tool_main_nf = os.path.join(self.directory, "modules", self.tool, "main.nf")
parent_tool_test_nf = os.path.join(self.directory, "tests", "modules", self.tool, "main.nf")
parent_tool_main_nf = os.path.join(self.directory, self.default_modules_path, self.tool, "main.nf")
parent_tool_test_nf = os.path.join(self.directory, self.default_tests_path, self.tool, "main.nf")
if self.subtool and os.path.exists(parent_tool_main_nf):
raise UserWarning(
f"Module '{parent_tool_main_nf}' exists already, cannot make subtool '{self.tool_name}'"
Expand All @@ -346,7 +349,7 @@ def get_module_dirs(self):
)

# If no subtool, check that there isn't already a tool/subtool
tool_glob = glob.glob(f"{os.path.join(self.directory, 'modules', self.tool)}/*/main.nf")
tool_glob = glob.glob(f"{os.path.join(self.directory, self.default_modules_path, self.tool)}/*/main.nf")
if not self.subtool and tool_glob:
raise UserWarning(
f"Module subtool '{tool_glob[0]}' exists already, cannot make tool '{self.tool_name}'"
Expand Down
17 changes: 10 additions & 7 deletions nf_core/modules/info.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ def init_mod_name(self, module):
if self.repo_type == "modules":
modules = self.get_modules_clone_modules()
else:
modules = self.modules_json.get_all_modules().get(self.modules_repo.fullname)
modules = self.modules_json.get_all_modules().get(self.modules_repo.remote_url)
modules = [module if dir == "nf-core" else f"{dir}/{module}" for dir, module in modules]
Copy link
Member

Choose a reason for hiding this comment

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

Reading quickly so can't see the full context, but why do we need special treatment for nf-core here?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is to get the module name suggestions, it will usually be nf-core, so we don't need to have nf-core/TOOL, but in case you have modules from other repos I thought it would be nice to differentiate and have the other directory listed.

if modules is None:
raise UserWarning(f"No modules installed from '{self.modules_repo.remote_url}'")
else:
Expand Down Expand Up @@ -135,15 +136,17 @@ def get_local_yaml(self):

if self.repo_type == "pipeline":
# Try to find and load the meta.yml file
repo_name = self.modules_repo.fullname
module_base_path = os.path.join(self.dir, "modules", repo_name)
repo_name = self.modules_repo.repo_path
module_base_path = os.path.join(self.dir, "modules")
# Check that we have any modules installed from this repo
modules = self.modules_json.get_all_modules().get(repo_name)
modules = self.modules_json.get_all_modules().get(self.modules_repo.remote_url)
module_names = [module for _, module in modules]
if modules is None:
raise LookupError(f"No modules installed from {self.modules_repo.remote_url}")

if self.module in modules:
mod_dir = os.path.join(module_base_path, self.module)
if self.module in module_names:
install_dir = [dir for dir, module in modules if module == self.module][0]
mod_dir = os.path.join(module_base_path, install_dir, self.module)
meta_fn = os.path.join(mod_dir, "meta.yml")
if os.path.exists(meta_fn):
log.debug(f"Found local file: {meta_fn}")
Expand All @@ -153,7 +156,7 @@ def get_local_yaml(self):

log.debug(f"Module '{self.module}' meta.yml not found locally")
else:
module_base_path = os.path.join(self.dir, "modules")
module_base_path = os.path.join(self.dir, "modules", "nf-core")
Copy link
Member

Choose a reason for hiding this comment

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

As above, hardcoded nf-core strings make me nervous 😅

if self.module in os.listdir(module_base_path):
mod_dir = os.path.join(module_base_path, self.module)
meta_fn = os.path.join(mod_dir, "meta.yml")
Expand Down
22 changes: 17 additions & 5 deletions nf_core/modules/install.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def install(self, module):
# Verify that the provided SHA exists in the repo
if self.sha:
if not self.modules_repo.sha_exists_on_branch(self.sha):
log.error(f"Commit SHA '{self.sha}' doesn't exist in '{self.modules_repo.fullname}'")
log.error(f"Commit SHA '{self.sha}' doesn't exist in '{self.modules_repo.remote_url}'")
return False

if module is None:
Expand All @@ -71,10 +71,12 @@ def install(self, module):
log.warning(warn_msg)
return False

current_version = modules_json.get_module_version(module, self.modules_repo.fullname)
current_version = modules_json.get_module_version(
module, self.modules_repo.remote_url, self.modules_repo.repo_path
)

# Set the install folder based on the repository name
install_folder = os.path.join(self.dir, "modules", self.modules_repo.fullname)
install_folder = os.path.join(self.dir, "modules", self.modules_repo.repo_path)

# Compute the module directory
module_dir = os.path.join(install_folder, module)
Expand All @@ -84,7 +86,7 @@ def install(self, module):

log.error("Module is already installed.")
repo_flag = (
"" if self.modules_repo.fullname == NF_CORE_MODULES_NAME else f"-g {self.modules_repo.remote_url} "
"" if self.modules_repo.repo_path == NF_CORE_MODULES_NAME else f"-g {self.modules_repo.remote_url} "
)
branch_flag = "" if self.modules_repo.branch == "master" else f"-b {self.modules_repo.branch} "

Expand All @@ -110,8 +112,18 @@ def install(self, module):
version = self.modules_repo.get_latest_module_version(module)

if self.force:
log.info(f"Removing installed version of '{self.modules_repo.fullname}/{module}'")
log.info(f"Removing installed version of '{self.modules_repo.repo_path}/{module}'")
self.clear_module_dir(module, module_dir)
for repo_url, repo_content in modules_json.modules_json["repos"].items():
for dir, dir_modules in repo_content["modules"].items():
for name, _ in dir_modules.items():
if name == module and dir == self.modules_repo.repo_path:
repo_to_remove = repo_url
log.info(
f"Removing module '{self.modules_repo.repo_path}/{module}' from repo '{repo_to_remove}' from modules.json"
)
modules_json.remove_entry(module, repo_to_remove, self.modules_repo.repo_path)
break

log.info(f"{'Rei' if self.force else 'I'}nstalling '{module}'")
log.debug(f"Installing module '{module}' at modules hash {version} from {self.modules_repo.remote_url}")
Expand Down
14 changes: 7 additions & 7 deletions nf_core/modules/lint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -85,12 +85,12 @@ def __init__(
modules_json = ModulesJson(self.dir)
modules_json.check_up_to_date()
all_pipeline_modules = modules_json.get_all_modules()
if self.modules_repo.fullname in all_pipeline_modules:
module_dir = Path(self.dir, "modules", self.modules_repo.fullname)
if self.modules_repo.remote_url in all_pipeline_modules:
module_dir = Path(self.dir, "modules", "nf-core")
self.all_remote_modules = [
NFCoreModule(m, self.modules_repo.fullname, module_dir / m, self.repo_type, Path(self.dir))
for m in all_pipeline_modules[self.modules_repo.fullname]
]
NFCoreModule(m[1], self.modules_repo.remote_url, module_dir / m[1], self.repo_type, Path(self.dir))
for m in all_pipeline_modules[self.modules_repo.remote_url]
] # m = (module_dir, module_name)
if not self.all_remote_modules:
raise LookupError(f"No modules from {self.modules_repo.remote_url} installed in pipeline.")
local_module_dir = Path(self.dir, "modules", "local")
Expand All @@ -102,7 +102,7 @@ def __init__(
else:
raise LookupError(f"No modules from {self.modules_repo.remote_url} installed in pipeline.")
else:
module_dir = Path(self.dir, "modules")
module_dir = Path(self.dir, self.default_modules_path)
self.all_remote_modules = [
NFCoreModule(m, None, module_dir / m, self.repo_type, Path(self.dir))
for m in self.get_modules_clone_modules()
Expand Down Expand Up @@ -144,7 +144,7 @@ def lint(
Lint all or one specific module

First gets a list of all local modules (in modules/local/process) and all modules
installed from nf-core (in modules/nf-core/modules)
installed from nf-core (in modules/nf-core)

For all nf-core modules, the correct file structure is assured and important
file content is verified. If directory subject to linting is a clone of 'nf-core/modules',
Expand Down
2 changes: 1 addition & 1 deletion nf_core/modules/lint/main_nf.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def main_nf(module_lint_object, module, fix_version, progress_bar):
if module.is_patched:
lines = ModulesDiffer.try_apply_patch(
module.module_name,
module_lint_object.modules_repo.fullname,
module_lint_object.modules_repo.repo_path,
module.patch_path,
Path(module.module_dir).relative_to(module.base_dir),
reverse=True,
Expand Down
2 changes: 1 addition & 1 deletion nf_core/modules/lint/meta_yml.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def meta_yml(module_lint_object, module):
if module.is_patched:
lines = ModulesDiffer.try_apply_patch(
module.module_name,
module_lint_object.modules_repo.fullname,
module_lint_object.modules_repo.repo_path,
module.patch_path,
Path(module.module_dir).relative_to(module.base_dir),
reverse=True,
Expand Down
2 changes: 1 addition & 1 deletion nf_core/modules/lint/module_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def module_changes(module_lint_object, module):
shutil.copytree(module.module_dir, tempdir)
try:
new_lines = ModulesDiffer.try_apply_patch(
module.module_name, module_lint_object.modules_repo.fullname, module.patch_path, tempdir, reverse=True
module.module_name, module_lint_object.modules_repo.repo_path, module.patch_path, tempdir, reverse=True
)
for file, lines in new_lines.items():
with open(tempdir / file, "w") as fh:
Expand Down
2 changes: 1 addition & 1 deletion nf_core/modules/lint/module_patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ def patch_reversible(module_lint_object, module, patch_path):
try:
ModulesDiffer.try_apply_patch(
module.module_name,
module_lint_object.modules_repo.fullname,
module_lint_object.modules_repo.repo_path,
patch_path,
Path(module.module_dir).relative_to(module.base_dir),
reverse=True,
Expand Down
2 changes: 1 addition & 1 deletion nf_core/modules/lint/module_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ def module_version(module_lint_object, module):

# Verify that a git_sha exists in the `modules.json` file for this module
version = module_lint_object.modules_json.get_module_version(
module.module_name, module_lint_object.modules_repo.fullname
module.module_name, module_lint_object.modules_repo.remote_url, module_lint_object.modules_repo.repo_path
)
if version is None:
module.failed.append(("git_sha", "No git_sha entry in `modules.json`", modules_json_path))
Expand Down
Loading