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

Make nf-core modules update --save-diff work when files are created or removed #1698

Merged
merged 10 commits into from
Jul 26, 2022

Conversation

ErikDanielsson
Copy link
Contributor

@ErikDanielsson ErikDanielsson commented Jul 25, 2022

As I reported in #1694, nf-core modules update --save-file simply ignored files that were created or removed between updates. I've fixed this by letting the output .diff file be more similar to the output from git diff:

  • Created or removed files are shown to originate from/end up in /dev/null
  • "from" and "to" file paths are prefixed with a/ and b/ respectively.

I've moved the diff methods in update to a separate static class to make the update code a bit cleaner also

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 25, 2022

Codecov Report

Merging #1698 (88c29a1) into dev (d47574f) will decrease coverage by 0.00%.
The diff coverage is 68.64%.

@@            Coverage Diff             @@
##              dev    #1698      +/-   ##
==========================================
- Coverage   68.24%   68.23%   -0.01%     
==========================================
  Files          56       57       +1     
  Lines        6694     6746      +52     
==========================================
+ Hits         4568     4603      +35     
- Misses       2126     2143      +17     
Impacted Files Coverage Δ
nf_core/modules/update.py 73.14% <52.63%> (-0.62%) ⬇️
nf_core/modules/modules_differ.py 71.71% <71.71%> (ø)

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 d47574f...88c29a1. Read the comment docs.

@ErikDanielsson ErikDanielsson requested a review from mirpedrol July 25, 2022 13:04
@ErikDanielsson ErikDanielsson changed the title Make nf-core modules update --save-diff work when files where created or removed Make nf-core modules update --save-diff work when files are created or removed Jul 25, 2022
Copy link
Member

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

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

I think it looks good! I found a small error while testing the command that should be addressed.

Also, I've seen that when I create a file to test what happens when a file needs to be removed, it works fine with --preview or updating directly, but it's not removed with the option --save-diff.
To reproduce:

git clone https://github.com/nf-core/rnaseq.git
cd rnaseq
nf-core modules update cat/fastq --sha 49da8642876ae4d91128168cd0db4f1c858d7792


touch modules/nf-core/modules/cat/fastq/test.yml
nf-core modules update --save-diff diff.file cat/fastq
grep "test.yml" diff.file
# test.yml file is not listed in diff.file
# Note that functions.py file is indeed removed as expected

nf-core modules update --preview cat/fastq
# test.yml file is listed in the preview and removed

If you think this is out of this PR scope, we can take a look in a different one :)

nf_core/modules/modules_differ.py Show resolved Hide resolved
nf_core/modules/modules_differ.py Show resolved Hide resolved
nf_core/modules/modules_differ.py Outdated Show resolved Hide resolved
@ErikDanielsson
Copy link
Contributor Author

ErikDanielsson commented Jul 26, 2022

Good points, I've added recursive checking of the module directory now.

Regarding the your first example: The issue here is that the test.yml file is empty, and identical to /dev/null; there is therefore no diff to generate. I tried adding things like

--- modules/nf-core/modules/cat/fastq/test.yml
+++ /dev/null
@@ -0,0 +0,0 @@

to try to coerce git apply into removing the file, but then it only reports that the patch is corrupt. So I think we'll have to accept that empty files can't be removed by the command.

@mirpedrol
Copy link
Member

I see the problem. Let's assume we won't usually have empty files :)

@ErikDanielsson
Copy link
Contributor Author

Sounds good! If it becomes a major issue somewhere, we can have a second look at it.

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.

Running nf-core modules update --save-diff when file was created or removed
2 participants