Make `nf-core modules` commands work with arbitrary git remotes #1724

ErikDanielsson · 2022-08-03T08:32:16Z

As @awgymer reported in #1721, several of the nf-core modules commands depended on that git remotes had the same format as in GitHub, i.e. owner/repo. This meant that the commands failed when repo names of greater nesting were used, for example group/subgroup/repo.

I've fixed this by letting the modules.json file be the only source of information for what modules are installed in a pipeline -- the commands should not need to interact with the directory structure directly.

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 · 2022-08-03T08:38:43Z

Codecov Report

Merging #1724 (b88a423) into dev (a9d0665) will decrease coverage by 0.06%.
The diff coverage is 70.14%.

@@            Coverage Diff             @@
##              dev    #1724      +/-   ##
==========================================
- Coverage   68.29%   68.23%   -0.07%     
==========================================
  Files          59       59              
  Lines        7063     7071       +8     
==========================================
+ Hits         4824     4825       +1     
- Misses       2239     2246       +7

Impacted Files	Coverage Δ
nf_core/modules/list.py	`71.23% <ø> (-0.39%)`	⬇️
nf_core/modules/info.py	`17.16% <12.50%> (+0.24%)`	⬆️
nf_core/modules/module_test.py	`53.33% <50.00%> (-1.47%)`	⬇️
nf_core/modules/modules_command.py	`84.00% <60.00%> (-3.81%)`	⬇️
nf_core/lint/modules_json.py	`80.76% <80.00%> (+0.76%)`	⬆️
nf_core/modules/modules_json.py	`65.52% <80.00%> (+0.78%)`	⬆️
nf_core/modules/lint/__init__.py	`82.98% <87.50%> (+0.01%)`	⬆️
nf_core/modules/install.py	`72.36% <100.00%> (-0.36%)`	⬇️
nf_core/modules/patch.py	`81.48% <100.00%> (ø)`
nf_core/modules/remove.py	`83.87% <100.00%> (+3.31%)`	⬆️
... and 1 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

nf_core/modules/info.py

awgymer · 2022-08-03T09:34:52Z

When trying --branch non-standard-path (which I have just created and pushed to test base-path) I am getting:

ERROR    Branch 'non-standard-path' not found in '<repo>'

awgymer · 2022-08-03T09:53:11Z

FWIW the <repo> in the error message above is just the dirname part? i.e. org/group/name without any url parts

ErikDanielsson · 2022-08-03T09:59:20Z

When trying --branch non-standard-path (which I have just created and pushed to test base-path) I am getting:
ERROR    Branch 'non-standard-path' not found in '<repo>'

Are you able to create and work with a branch where you've not changed the base-path?

awgymer · 2022-08-03T10:01:48Z

So I did some debugging and when it was doing the call to branch_exists, self.repo.branches contained only master.
I had to add a self.repo.git.fetch before the checkout and it worked fine. I suspect this may be a caching issue or something?

awgymer · 2022-08-03T10:09:45Z

Not sure if this is intended/expected, and may be outside the scope of this PR, but list with non-standard path/branch has some issues.

modules list local

...

WARNING  Repository '<repo>' (master) does not contain the 'othermods' directory

Message and Date are listed as Not Available in the table.
Passing both the branch and path gave the same error.

Leaving out the path resulted in:

CRITICAL Repository '<repo>' (non-standard-path) does not contain the 'modules' directory

ErikDanielsson · 2022-08-03T11:06:26Z

Yes, put it in an issue and I'll have a look at it this afternoon

ErikDanielsson · 2022-08-03T13:27:05Z

The issues mentioned should've been address elsewhere, so this PR should be ready for review now

mirpedrol

The code looks good to me, only a couple of suggestions :)
I am not sure how to test this functionality, so if @awgymer is happy with it I think we can merge

nf_core/modules/modules_command.py

mirpedrol · 2022-08-03T14:14:54Z

nf_core/modules/remove.py

        if module is None:
            module = questionary.autocomplete(
-                "Tool name:", choices=self.module_names[repo_name], style=nf_core.utils.nfcore_question_style
+                "Tool name:",
+                choices=self.modules_from_repo(repo_name),


I think this line should change in the other commands also, is that right?
For example the one from patch.py is failing

Yes you are right, thanks! Though it seems like patch.py was the only one I missed, I've updated it now.

Co-authored-by: Júlia Mir Pedrol <mirp.julia@gmail.com>

awgymer · 2022-08-03T15:01:25Z

I am not sure how to test this functionality, so if @awgymer is happy with it I think we can merge

The things I have tried so far all seem to be working.

ErikDanielsson · 2022-08-03T15:03:13Z

Thanks @awgymer for testing and @mirpedrol for reviewing!

ErikDanielsson added 6 commits August 2, 2022 19:16

Make nf-core modules list local agnostic. Fix dir_tree_uncovered

5695c61

Fix patch and remove

4823d5e

Update and info

8b5221d

modules_json lint modules_test and remove method

969e7db

Fix failing test

eb02224

Update changelog

c23690d

ErikDanielsson changed the title ~~Make nf-core modules command work with repostories of greater nesting~~ Make nf-core modules command work with arbitrary git remotes Aug 3, 2022

ErikDanielsson changed the title ~~Make nf-core modules command work with arbitrary git remotes~~ Make nf-core modules commands work with arbitrary git remotes Aug 3, 2022

ErikDanielsson mentioned this pull request Aug 3, 2022

modules list doesn't work for custom remotes with a subproject #1721

Closed

ErikDanielsson linked an issue Aug 3, 2022 that may be closed by this pull request

modules list doesn't work for custom remotes with a subproject #1721

Closed

ErikDanielsson requested a review from mirpedrol August 3, 2022 08:45

ErikDanielsson force-pushed the git-remote-agnostic branch from fe31bc8 to c23690d Compare August 3, 2022 09:06

awgymer reviewed Aug 3, 2022

View reviewed changes

nf_core/modules/info.py Show resolved Hide resolved

Fix bug in info

623ce30

Merge branch 'dev' of github.com:nf-core/tools into git-remote-agnostic

b0b0bed

Fix failing lint tests

7048db7

ErikDanielsson mentioned this pull request Aug 3, 2022

Fix issues related to --branch and --base-path #1727

Merged

4 tasks

mirpedrol approved these changes Aug 3, 2022

View reviewed changes

ErikDanielsson and others added 2 commits August 3, 2022 16:35

Update nf_core/modules/modules_command.py

f80a5c9

Co-authored-by: Júlia Mir Pedrol <mirp.julia@gmail.com>

Fix patch

b88a423

ErikDanielsson merged commit da1f79f into nf-core:dev Aug 3, 2022

ErikDanielsson deleted the git-remote-agnostic branch August 3, 2022 15:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make `nf-core modules` commands work with arbitrary git remotes #1724

Make `nf-core modules` commands work with arbitrary git remotes #1724

ErikDanielsson commented Aug 3, 2022

codecov bot commented Aug 3, 2022 •

edited

Loading

awgymer commented Aug 3, 2022

awgymer commented Aug 3, 2022

ErikDanielsson commented Aug 3, 2022

awgymer commented Aug 3, 2022

awgymer commented Aug 3, 2022

ErikDanielsson commented Aug 3, 2022

ErikDanielsson commented Aug 3, 2022

mirpedrol left a comment

mirpedrol Aug 3, 2022

ErikDanielsson Aug 3, 2022

awgymer commented Aug 3, 2022 •

edited

Loading

ErikDanielsson commented Aug 3, 2022

Make nf-core modules commands work with arbitrary git remotes #1724

Make nf-core modules commands work with arbitrary git remotes #1724

Conversation

ErikDanielsson commented Aug 3, 2022

PR checklist

codecov bot commented Aug 3, 2022 • edited Loading

Codecov Report

awgymer commented Aug 3, 2022

awgymer commented Aug 3, 2022

ErikDanielsson commented Aug 3, 2022

awgymer commented Aug 3, 2022

awgymer commented Aug 3, 2022

ErikDanielsson commented Aug 3, 2022

ErikDanielsson commented Aug 3, 2022

mirpedrol left a comment

Choose a reason for hiding this comment

mirpedrol Aug 3, 2022

Choose a reason for hiding this comment

ErikDanielsson Aug 3, 2022

Choose a reason for hiding this comment

awgymer commented Aug 3, 2022 • edited Loading

ErikDanielsson commented Aug 3, 2022

Make `nf-core modules` commands work with arbitrary git remotes #1724

Make `nf-core modules` commands work with arbitrary git remotes #1724

codecov bot commented Aug 3, 2022 •

edited

Loading

awgymer commented Aug 3, 2022 •

edited

Loading