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

🩹 added patch command for subworkflows #2861

Merged
merged 29 commits into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
7d24e75
added patch command for subworkflows
ctuni Mar 18, 2024
ef7371f
Merge branch 'dev' into master
ctuni Mar 18, 2024
86e3e2f
forgot to import patch in init
ctuni Mar 18, 2024
9a0b2e7
Merge branch 'master' of https://github.com/ctuni/tools
ctuni Mar 18, 2024
26b39c2
added files for the tests
ctuni Mar 18, 2024
015a61b
created draft for test file
ctuni Mar 18, 2024
563e232
ruff format
ctuni Mar 18, 2024
31505ab
cleaning up
ctuni Mar 18, 2024
4d0dc85
added tests
ctuni Mar 18, 2024
3030a76
ruff
ctuni Mar 18, 2024
6b885ff
ruff format
ctuni Mar 18, 2024
dfcfb5b
removed split
ctuni Mar 18, 2024
2991930
mypy
ctuni Mar 18, 2024
e909d3c
setup_patch
ctuni Mar 18, 2024
fd5b0d1
called function correctly
ctuni Mar 18, 2024
cdd9cfb
wraping up for the day
ctuni Mar 18, 2024
7f40b3a
Merge branch 'dev' of https://github.com/nf-core/tools
mirpedrol Nov 15, 2024
7cd3dea
update and fix swf patch tests
mirpedrol Nov 15, 2024
424bbae
Merge branch 'dev' into master
ewels Nov 21, 2024
25940f7
apply patch reverse when linting a patched subworkflow
mirpedrol Nov 22, 2024
b2cfd01
update get_patch_fn to work with subworkflows
mirpedrol Nov 22, 2024
6ec2dcd
move modules_differ.py to components_differ.py
mirpedrol Nov 22, 2024
27582f9
add subworkflows patch missing tests
mirpedrol Nov 25, 2024
4f93d57
fix subworkflows update test
mirpedrol Nov 25, 2024
5ad4570
update changelog
mirpedrol Nov 25, 2024
37ca244
add help text for --remove flag
mirpedrol Nov 25, 2024
805ba91
apply code review suggestions to patch tests
mirpedrol Nov 25, 2024
84cec26
Update tests/modules/test_patch.py
mirpedrol Nov 25, 2024
8b2bec4
apply suggestions by @mashehu
mirpedrol Nov 25, 2024
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: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

### Subworkflows

- Add `nf-core subworkflows patch` command ([#2861](https://github.com/nf-core/tools/pull/2861))

### General

- Include .nf-core.yml in `nf-core pipelines bump-version` ([#3220](https://github.com/nf-core/tools/pull/3220))
Expand Down
39 changes: 38 additions & 1 deletion nf_core/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1024,7 +1024,7 @@ def command_modules_update(
default=".",
help=r"Pipeline directory. [dim]\[default: current working directory][/]",
)
@click.option("-r", "--remove", is_flag=True, default=False)
@click.option("-r", "--remove", is_flag=True, default=False, help="Remove an existent patch file and regenerate it.")
def command_modules_patch(ctx, tool, directory, remove):
"""
Create a patch file for minor changes in a module
Expand Down Expand Up @@ -1567,6 +1567,43 @@ def command_subworkflows_install(ctx, subworkflow, directory, prompt, force, sha
subworkflows_install(ctx, subworkflow, directory, prompt, force, sha)


# nf-core subworkflows patch
@subworkflows.command("patch")
@click.pass_context
@click.argument("tool", type=str, required=False, metavar="<tool> or <tool/subtool>")
@click.option(
"-d",
"--dir",
type=click.Path(exists=True),
default=".",
help=r"Pipeline directory. [dim]\[default: current working directory][/]",
)
@click.option("-r", "--remove", is_flag=True, default=False, help="Remove an existent patch file and regenerate it.")
def subworkflows_patch(ctx, tool, dir, remove):
"""
Create a patch file for minor changes in a subworkflow

Checks if a subworkflow has been modified locally and creates a patch file
describing how the module has changed from the remote version
"""
from nf_core.subworkflows import SubworkflowPatch

try:
subworkflow_patch = SubworkflowPatch(
dir,
ctx.obj["modules_repo_url"],
ctx.obj["modules_repo_branch"],
ctx.obj["modules_repo_no_pull"],
)
if remove:
subworkflow_patch.remove(tool)
else:
subworkflow_patch.patch(tool)
except (UserWarning, LookupError) as e:
log.error(e)
sys.exit(1)


# nf-core subworkflows remove
@subworkflows.command("remove")
@click.pass_context
Expand Down

Large diffs are not rendered by default.

20 changes: 13 additions & 7 deletions nf_core/components/patch.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import nf_core.utils
from nf_core.components.components_command import ComponentCommand
from nf_core.modules.modules_differ import ModulesDiffer
from nf_core.components.components_differ import ComponentsDiffer
from nf_core.modules.modules_json import ModulesJson

log = logging.getLogger(__name__)
Expand Down Expand Up @@ -65,7 +65,9 @@ def patch(self, component=None):
component_fullname = str(Path(self.component_type, self.modules_repo.repo_path, component))

# Verify that the component has an entry in the modules.json file
if not self.modules_json.module_present(component, self.modules_repo.remote_url, component_dir):
if not self.modules_json.component_present(
component, self.modules_repo.remote_url, component_dir, self.component_type
):
raise UserWarning(
f"The '{component_fullname}' {self.component_type[:-1]} does not have an entry in the 'modules.json' file. Cannot compute patch"
)
Expand Down Expand Up @@ -112,7 +114,7 @@ def patch(self, component=None):
# Write the patch to a temporary location (otherwise it is printed to the screen later)
patch_temp_path = tempfile.mktemp()
try:
ModulesDiffer.write_diff_file(
ComponentsDiffer.write_diff_file(
patch_temp_path,
component,
self.modules_repo.repo_path,
Expand All @@ -127,11 +129,13 @@ def patch(self, component=None):
raise UserWarning(f"{self.component_type[:-1]} '{component_fullname}' is unchanged. No patch to compute")

# Write changes to modules.json
self.modules_json.add_patch_entry(component, self.modules_repo.remote_url, component_dir, patch_relpath)
self.modules_json.add_patch_entry(
self.component_type, component, self.modules_repo.remote_url, component_dir, patch_relpath
)
log.debug(f"Wrote patch path for {self.component_type[:-1]} {component} to modules.json")

# Show the changes made to the module
ModulesDiffer.print_diff(
ComponentsDiffer.print_diff(
component,
self.modules_repo.repo_path,
component_install_dir,
Expand Down Expand Up @@ -166,7 +170,9 @@ def remove(self, component):
component_fullname = str(Path(self.component_type, component_dir, component))

# Verify that the component has an entry in the modules.json file
if not self.modules_json.module_present(component, self.modules_repo.remote_url, component_dir):
if not self.modules_json.component_present(
component, self.modules_repo.remote_url, component_dir, self.component_type
):
raise UserWarning(
f"The '{component_fullname}' {self.component_type[:-1]} does not have an entry in the 'modules.json' file. Cannot compute patch"
)
Expand Down Expand Up @@ -202,7 +208,7 @@ def remove(self, component):

# Try to apply the patch in reverse and move resulting files to module dir
temp_component_dir = self.modules_json.try_apply_patch_reverse(
component, self.modules_repo.repo_path, patch_relpath, component_path
self.component_type, component, self.modules_repo.repo_path, patch_relpath, component_path
)
try:
for file in Path(temp_component_dir).glob("*"):
Expand Down
2 changes: 1 addition & 1 deletion nf_core/components/remove.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ def remove(self, component, removed_by=None, removed_components=None, force=Fals
if not component_dir.exists():
log.error(f"Installation directory '{component_dir}' does not exist.")

if modules_json.module_present(component, self.modules_repo.remote_url, repo_path):
if modules_json.component_present(component, self.modules_repo.remote_url, repo_path, self.component_type):
log.error(f"Found entry for '{component}' in 'modules.json'. Removing...")
modules_json.remove_entry(self.component_type, component, self.modules_repo.remote_url, repo_path)
return False
Expand Down
32 changes: 23 additions & 9 deletions nf_core/components/update.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
import nf_core.modules.modules_utils
import nf_core.utils
from nf_core.components.components_command import ComponentCommand
from nf_core.components.components_differ import ComponentsDiffer
from nf_core.components.components_utils import (
get_components_to_install,
prompt_component_version_sha,
)
from nf_core.components.install import ComponentInstall
from nf_core.components.remove import ComponentRemove
from nf_core.modules.modules_differ import ModulesDiffer
from nf_core.modules.modules_json import ModulesJson
from nf_core.modules.modules_repo import ModulesRepo
from nf_core.utils import plural_es, plural_s, plural_y
Expand Down Expand Up @@ -223,7 +223,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr
f"Writing diff file for {self.component_type[:-1]} '{component_fullname}' to '{self.save_diff_fn}'"
)
try:
ModulesDiffer.write_diff_file(
ComponentsDiffer.write_diff_file(
self.save_diff_fn,
component,
modules_repo.repo_path,
Expand Down Expand Up @@ -265,7 +265,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr
self.manage_changes_in_linked_components(component, modules_to_update, subworkflows_to_update)

elif self.show_diff:
ModulesDiffer.print_diff(
ComponentsDiffer.print_diff(
component,
modules_repo.repo_path,
component_dir,
Expand Down Expand Up @@ -313,7 +313,7 @@ def update(self, component=None, silent=False, updated=None, check_diff_exist=Tr

if self.save_diff_fn:
# Write the modules.json diff to the file
ModulesDiffer.append_modules_json_diff(
ComponentsDiffer.append_modules_json_diff(
self.save_diff_fn,
old_modules_json,
self.modules_json.get_modules_json(),
Expand Down Expand Up @@ -449,7 +449,9 @@ def get_single_component_info(self, component):
self.modules_repo.setup_branch(current_branch)

# If there is a patch file, get its filename
patch_fn = self.modules_json.get_patch_fn(component, self.modules_repo.remote_url, install_dir)
patch_fn = self.modules_json.get_patch_fn(
self.component_type, component, self.modules_repo.remote_url, install_dir
)

return (self.modules_repo, component, sha, patch_fn)

Expand Down Expand Up @@ -695,7 +697,12 @@ def get_all_components_info(self, branch=None):

# Add patch filenames to the components that have them
components_info = [
(repo, comp, sha, self.modules_json.get_patch_fn(comp, repo.remote_url, repo.repo_path))
(
repo,
comp,
sha,
self.modules_json.get_patch_fn(self.component_type, comp, repo.remote_url, repo.repo_path),
)
for repo, comp, sha in components_info
]

Expand Down Expand Up @@ -810,7 +817,9 @@ def try_apply_patch(
shutil.copytree(component_install_dir, temp_component_dir)

try:
new_files = ModulesDiffer.try_apply_patch(component, repo_path, patch_path, temp_component_dir)
new_files = ComponentsDiffer.try_apply_patch(
self.component_type, component, repo_path, patch_path, temp_component_dir
)
except LookupError:
# Patch failed. Save the patch file by moving to the install dir
shutil.move(patch_path, Path(component_install_dir, patch_path.relative_to(component_dir)))
Expand All @@ -828,7 +837,7 @@ def try_apply_patch(

# Create the new patch file
log.debug("Regenerating patch file")
ModulesDiffer.write_diff_file(
ComponentsDiffer.write_diff_file(
Path(temp_component_dir, patch_path.relative_to(component_dir)),
component,
repo_path,
Expand All @@ -848,7 +857,12 @@ def try_apply_patch(

# Add the patch file to the modules.json file
self.modules_json.add_patch_entry(
component, self.modules_repo.remote_url, repo_path, patch_relpath, write_file=write_file
self.component_type,
component,
self.modules_repo.remote_url,
repo_path,
patch_relpath,
write_file=write_file,
)

return True
Expand Down
5 changes: 3 additions & 2 deletions nf_core/modules/lint/main_nf.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@

import nf_core
import nf_core.modules.modules_utils
from nf_core.components.components_differ import ComponentsDiffer
from nf_core.components.nfcore_component import NFCoreComponent
from nf_core.modules.modules_differ import ModulesDiffer

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -50,7 +50,8 @@ def main_nf(
# otherwise read the lines directly from the module
lines: List[str] = []
if module.is_patched:
lines = ModulesDiffer.try_apply_patch(
lines = ComponentsDiffer.try_apply_patch(
module.component_type,
module.component_name,
module_lint_object.modules_repo.repo_path,
module.patch_path,
Expand Down
8 changes: 5 additions & 3 deletions nf_core/modules/lint/meta_yml.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
import ruamel.yaml
from jsonschema import exceptions, validators

from nf_core.components.components_differ import ComponentsDiffer
from nf_core.components.lint import ComponentLint, LintExceptionError
from nf_core.components.nfcore_component import NFCoreComponent
from nf_core.modules.modules_differ import ModulesDiffer

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -46,7 +46,8 @@ def meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) -> None
# Check if we have a patch file, get original file in that case
meta_yaml = read_meta_yml(module_lint_object, module)
if module.is_patched and module_lint_object.modules_repo.repo_path is not None:
lines = ModulesDiffer.try_apply_patch(
lines = ComponentsDiffer.try_apply_patch(
module.component_type,
module.component_name,
module_lint_object.modules_repo.repo_path,
module.patch_path,
Expand Down Expand Up @@ -207,7 +208,8 @@ def read_meta_yml(module_lint_object: ComponentLint, module: NFCoreComponent) ->
yaml.preserve_quotes = True
# Check if we have a patch file, get original file in that case
if module.is_patched:
lines = ModulesDiffer.try_apply_patch(
lines = ComponentsDiffer.try_apply_patch(
module.component_type,
module.component_name,
module_lint_object.modules_repo.repo_path,
module.patch_path,
Expand Down
5 changes: 3 additions & 2 deletions nf_core/modules/lint/module_changes.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from pathlib import Path

import nf_core.modules.modules_repo
from nf_core.modules.modules_differ import ModulesDiffer
from nf_core.components.components_differ import ComponentsDiffer


def module_changes(module_lint_object, module):
Expand All @@ -30,7 +30,8 @@ def module_changes(module_lint_object, module):
tempdir = tempdir_parent / "tmp_module_dir"
shutil.copytree(module.component_dir, tempdir)
try:
new_lines = ModulesDiffer.try_apply_patch(
new_lines = ComponentsDiffer.try_apply_patch(
module.component_type,
module.component_name,
module.org,
module.patch_path,
Expand Down
17 changes: 9 additions & 8 deletions nf_core/modules/lint/module_patch.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
from pathlib import Path

from ...components.components_differ import ComponentsDiffer
from ...components.nfcore_component import NFCoreComponent
from ..modules_differ import ModulesDiffer


def module_patch(module_lint_obj, module: NFCoreComponent):
Expand Down Expand Up @@ -66,11 +66,11 @@ def check_patch_valid(module, patch_path):
continue
topath = Path(line.split(" ")[1].strip("\n"))
if frompath == Path("/dev/null"):
paths_in_patch.append((frompath, ModulesDiffer.DiffEnum.CREATED))
paths_in_patch.append((frompath, ComponentsDiffer.DiffEnum.CREATED))
elif topath == Path("/dev/null"):
paths_in_patch.append((frompath, ModulesDiffer.DiffEnum.REMOVED))
paths_in_patch.append((frompath, ComponentsDiffer.DiffEnum.REMOVED))
elif frompath == topath:
paths_in_patch.append((frompath, ModulesDiffer.DiffEnum.CHANGED))
paths_in_patch.append((frompath, ComponentsDiffer.DiffEnum.CHANGED))
else:
module.failed.append(
(
Expand Down Expand Up @@ -105,7 +105,7 @@ def check_patch_valid(module, patch_path):
# Warn about any created or removed files
passed = True
for path, diff_status in paths_in_patch:
if diff_status == ModulesDiffer.DiffEnum.CHANGED:
if diff_status == ComponentsDiffer.DiffEnum.CHANGED:
if not Path(module.base_dir, path).exists():
module.failed.append(
(
Expand All @@ -116,7 +116,7 @@ def check_patch_valid(module, patch_path):
)
passed = False
continue
elif diff_status == ModulesDiffer.DiffEnum.CREATED:
elif diff_status == ComponentsDiffer.DiffEnum.CREATED:
if not Path(module.base_dir, path).exists():
module.failed.append(
(
Expand All @@ -130,7 +130,7 @@ def check_patch_valid(module, patch_path):
module.warned.append(
("patch", f"Patch file performs file creation of {path}. This is discouraged."), patch_path
)
elif diff_status == ModulesDiffer.DiffEnum.REMOVED:
elif diff_status == ComponentsDiffer.DiffEnum.REMOVED:
if Path(module.base_dir, path).exists():
module.failed.append(
(
Expand Down Expand Up @@ -161,7 +161,8 @@ def patch_reversible(module_lint_object, module, patch_path):
(bool): False if any test failed, True otherwise
"""
try:
ModulesDiffer.try_apply_patch(
ComponentsDiffer.try_apply_patch(
module.component_type,
module.component_name,
module_lint_object.modules_repo.repo_path,
patch_path,
Expand Down
Loading
Loading