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

Add nf-core modules patch command #1708

Merged
merged 23 commits into from
Aug 1, 2022

Conversation

ErikDanielsson
Copy link
Contributor

I've added a nf-core modules patch command for generating patches for minor changes in modules as was suggested in #1312. It works by comparing the local installation of a module to the one remote version in the modules.json.

The tests are done using a special branch in the nf-core/modules-test in GitLab where I've added the bismark/align module.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@codecov
Copy link

codecov bot commented Jul 28, 2022

Codecov Report

Merging #1708 (b1aca50) into dev (d070fc6) will increase coverage by 0.35%.
The diff coverage is 79.04%.

@@            Coverage Diff             @@
##              dev    #1708      +/-   ##
==========================================
+ Coverage   68.25%   68.60%   +0.35%     
==========================================
  Files          57       58       +1     
  Lines        6744     6817      +73     
==========================================
+ Hits         4603     4677      +74     
+ Misses       2141     2140       -1     
Impacted Files Coverage Δ
nf_core/__main__.py 52.66% <45.45%> (-0.19%) ⬇️
nf_core/modules/modules_differ.py 84.69% <80.00%> (+12.97%) ⬆️
nf_core/modules/patch.py 81.48% <81.48%> (ø)
nf_core/modules/modules_json.py 64.42% <85.00%> (+1.28%) ⬆️
nf_core/modules/update.py 75.60% <88.88%> (+2.46%) ⬆️
nf_core/modules/__init__.py 100.00% <100.00%> (ø)
nf_core/modules/test_yml_builder.py 48.16% <0.00%> (-0.46%) ⬇️
nf_core/utils.py 80.78% <0.00%> (-0.20%) ⬇️
nf_core/modules/modules_repo.py 82.16% <0.00%> (+0.63%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d070fc6...b1aca50. Read the comment docs.

@ErikDanielsson ErikDanielsson linked an issue Aug 1, 2022 that may be closed by this pull request
3 tasks
@mirpedrol
Copy link
Member

Looks good!
There is an error with the .diff file inside the module directory when the module is updated

╭──────────────────────────────────────────────────────── Traceback (most recent call last) ─────────────────────────────────────────────────────────╮
│ /opt/conda/bin/nf-core:33 in <module>                                                                                                              │
│                                                                                                                                                    │
│   32 │   sys.argv[0] = re.sub(r'(-script\.pyw?|\.exe)?$', '', sys.argv[0])                                                                         │
│ ❱ 33 │   sys.exit(load_entry_point('nf-core', 'console_scripts', 'nf-core')())                                                                     │
│   34                                                                                                                                               │
│                                                                                                                                                    │
│ /workspace/tools/nf_core/__main__.py:92 in run_nf_core                                                                                             │
│                                                                                                                                                    │
│     91 │   # Lanch the click cli                                                                                                                   │
│ ❱   92 │   nf_core_cli()                                                                                                                           │
│     93                                                                                                                                             │
│                                                                                                                                                    │
│ /opt/conda/lib/python3.9/site-packages/click/core.py:1130 in __call__                                                                              │
│                                                                                                                                                    │
│   1129 │   │   """Alias for :meth:`main`."""                                                                                                       │
│ ❱ 1130 │   │   return self.main(*args, **kwargs)                                                                                                   │
│   1131                                                                                                                                             │
│                                                                                                                                                    │
│ /opt/conda/lib/python3.9/site-packages/rich_click/rich_group.py:21 in main                                                                         │
│                                                                                                                                                    │
│   20 │   │   try:                                                                                                                                  │
│ ❱ 21 │   │   │   return super().main(*args, standalone_mode=False, **kwargs)                                                                       │
│   22 │   │   except click.ClickException as e:                                                                                                     │
│                                                                                                                                                    │
│ /opt/conda/lib/python3.9/site-packages/click/core.py:1055 in main                                                                                  │
│                                                                                                                                                    │
│   1054 │   │   │   │   with self.make_context(prog_name, args, **extra) as ctx:                                                                    │
│ ❱ 1055 │   │   │   │   │   rv = self.invoke(ctx)                                                                                                   │
│   1056 │   │   │   │   │   if not standalone_mode:                                                                                                 │
│                                                                                                                                                    │
│ /opt/conda/lib/python3.9/site-packages/click/core.py:1657 in invoke                                                                                │
│                                                                                                                                                    │
│   1656 │   │   │   │   with sub_ctx:                                                                                                               │
│ ❱ 1657 │   │   │   │   │   return _process_result(sub_ctx.command.invoke(sub_ctx))                                                                 │
│   1658                                                                                                                                             │
│                                                                                                                                                    │
│ /opt/conda/lib/python3.9/site-packages/click/core.py:1657 in invoke                                                                                │
│                                                                                                                                                    │
│   1656 │   │   │   │   with sub_ctx:                                                                                                               │
│ ❱ 1657 │   │   │   │   │   return _process_result(sub_ctx.command.invoke(sub_ctx))                                                                 │
│   1658                                                                                                                                             │
│                                                                                                                                                    │
│ /opt/conda/lib/python3.9/site-packages/click/core.py:1404 in invoke                                                                                │
│                                                                                                                                                    │
│   1403 │   │   if self.callback is not None:                                                                                                       │
│ ❱ 1404 │   │   │   return ctx.invoke(self.callback, **ctx.params)                                                                                  │
│   1405                                                                                                                                             │
│                                                                                                                                                    │
│ /opt/conda/lib/python3.9/site-packages/click/core.py:760 in invoke                                                                                 │
│                                                                                                                                                    │
│    759 │   │   │   with ctx:                                                                                                                       │
│ ❱  760 │   │   │   │   return __callback(*args, **kwargs)                                                                                          │
│    761                                                                                                                                             │
│                                                                                                                                                    │
│ /opt/conda/lib/python3.9/site-packages/click/decorators.py:26 in new_func                                                                          │
│                                                                                                                                                    │
│    25 │   def new_func(*args, **kwargs):  # type: ignore                                                                                           │
│ ❱  26 │   │   return f(get_current_context(), *args, **kwargs)                                                                                     │
│    27                                                                                                                                              │
│                                                                                                                                                    │
│ /workspace/tools/nf_core/__main__.py:533 in update                                                                                                 │
│                                                                                                                                                    │
│    532 │   │   )                                                                                                                                   │
│ ❱  533 │   │   exit_status = module_install.update(tool)                                                                                           │
│    534 │   │   if not exit_status and all:                                                                                                         │
│                                                                                                                                                    │
│ /workspace/tools/nf_core/modules/update.py:201 in update                                                                                           │
│                                                                                                                                                    │
│   200 │   │   │   │   elif self.show_diff:                                                                                                         │
│ ❱ 201 │   │   │   │   │   ModulesDiffer.print_diff(                                                                                                │
│   202 │   │   │   │   │   │   module, modules_repo.fullname, module_dir, module_install_dir,                                                       │
│       current_version, version                                                                                                                     │
│                                                                                                                                                    │
│ /workspace/tools/nf_core/modules/modules_differ.py:259 in print_diff                                                                               │
│                                                                                                                                                    │
│   258 │   │   │   │   # The file was removed between the commits                                                                                   │
│ ❱ 259 │   │   │   │   log.info(f"'{Path(dsp_from_dir, file)}' was removed")                                                                        │
│   260 │   │   │   else:                                                                                                                            │
│                                                                                                                                                    │
│ /opt/conda/lib/python3.9/pathlib.py:1082 in __new__                                                                                                │
│                                                                                                                                                    │
│   1081 │   │   │   cls = WindowsPath if os.name == 'nt' else PosixPath                                                                             │
│ ❱ 1082 │   │   self = cls._from_parts(args, init=False)                                                                                            │
│   1083 │   │   if not self._flavour.is_supported:                                                                                                  │
│                                                                                                                                                    │
│ /opt/conda/lib/python3.9/pathlib.py:707 in _from_parts                                                                                             │
│                                                                                                                                                    │
│    706 │   │   self = object.__new__(cls)                                                                                                          │
│ ❱  707 │   │   drv, root, parts = self._parse_args(args)                                                                                           │
│    708 │   │   self._drv = drv                                                                                                                     │
│                                                                                                                                                    │
│ /opt/conda/lib/python3.9/pathlib.py:691 in _parse_args                                                                                             │
│                                                                                                                                                    │
│    690 │   │   │   else:                                                                                                                           │
│ ❱  691 │   │   │   │   a = os.fspath(a)                                                                                                            │
│    692 │   │   │   │   if isinstance(a, str):                                                                                                      │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
TypeError: expected str, bytes or os.PathLike object, not NoneType

I think we could consider saving the .diff file in a different directory, inside modules for example. Also, the command git apply module.diff needs to be executed from inside the modules directory so I think it might be a good solution.

@ErikDanielsson
Copy link
Contributor Author

I believe (or hope ;)) that the update issue will be solved by #1710. I've split the PRs since they would've been too huge to review as one.

I did not think that this command should work with git apply, since the diff file shows the diff with the remote files which are unavailable -- every diff action should be handled by our own commands. Therefore file names are not shown in the git fashion, i.e. prefixed with a/ or b/. The main reason for keeping the diff file in the module's directory is that we don't flood any other directory with patch files, which I'm anxious will be the case if we keep them in the modules directory. I do agree however that the paths in the patch file is a bit misleading -- it should just be the filename.

@mirpedrol
Copy link
Member

I was seeing #1710 now :) I commented too fast!
I digged a bit more into the error and it only appears when I try to preview the differences.

@ErikDanielsson
Copy link
Contributor Author

Ok, then it should be solved by the next PR

@ErikDanielsson
Copy link
Contributor Author

Thanks for the review!

@ErikDanielsson ErikDanielsson merged commit 66c3faa into nf-core:dev Aug 1, 2022
@ErikDanielsson ErikDanielsson deleted the add-patch-command branch August 1, 2022 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DSL2 Modules: Support .diff files for minor changes
2 participants