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

Dev -> Master for v2.4 release #1581

Merged
merged 190 commits into from
May 16, 2022
Merged

Dev -> Master for v2.4 release #1581

merged 190 commits into from
May 16, 2022

Conversation

ewels
Copy link
Member

@ewels ewels commented May 13, 2022

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

ewels and others added 30 commits March 24, 2022 12:23
Noticed that when there are more dots in the FASTQ file name, the Path(file).suffixes returns all elements split by dot, which ends in list mismatch, therefore throwing an error. I think we should limit it to the last two items (according to VALID_FORMATS) for the extension check. Hope this PR helps to solve the problem.
Fix: Get file extensions correctly in validate_pair
Add a last newline to modules.json to avoid prettier error
Tower.nf is now running Nextflow 22.03.0.edge by default, which includes a new strategy to handle retrying jobs killed due to AWS spot policies. This was the reason for these lines of config, so they can now be removed.
Remove nextflow_config from AWS test CI
Direct copy of .gitignore.

Hopefully can get rid of this file again one day, see prettier/prettier#4708
Escaped test run output before logging it, to avoid a rich `MarkupError`
- Move the {% endraw %} statements off comment lines and on to the tail of the last statement to get rid of flating comment marker
- Remove quotes around the master branch check to try to fix action
GH Actions workflow YAML tweaks.
ewels and others added 14 commits May 13, 2022 13:44
Also check that only script or shell used and not both.
Module linting: Only lint script if there was a script.
Co-authored-by: Júlia Mir Pedrol <mirp.julia@gmail.com>
- Remove black backgrounds. Now that GitHub has dark mode, they dissapear into the background.
- Add a badge to launch on Tower
Module lint: Allow no input or output in YAML
@codecov
Copy link

codecov bot commented May 13, 2022

Codecov Report

Merging #1581 (414cc9a) into master (e0d00d3) will decrease coverage by 0.21%.
The diff coverage is 60.16%.

@@            Coverage Diff             @@
##           master    #1581      +/-   ##
==========================================
- Coverage   64.85%   64.64%   -0.22%     
==========================================
  Files          52       54       +2     
  Lines        6055     6253     +198     
==========================================
+ Hits         3927     4042     +115     
- Misses       2128     2211      +83     
Impacted Files Coverage Δ
nf_core/lint/files_exist.py 81.81% <ø> (ø)
nf_core/lint/files_unchanged.py 76.00% <ø> (ø)
nf_core/__main__.py 50.55% <34.48%> (-1.12%) ⬇️
nf_core/modules/module_utils.py 37.60% <45.45%> (-0.64%) ⬇️
nf_core/modules/module_test.py 48.27% <48.27%> (ø)
nf_core/launch.py 65.02% <50.00%> (+0.09%) ⬆️
nf_core/modules/test_yml_builder.py 48.40% <50.00%> (-0.90%) ⬇️
nf_core/modules/lint/meta_yml.py 64.10% <53.33%> (+1.94%) ⬆️
nf_core/lint/__init__.py 72.98% <60.00%> (+3.13%) ⬆️
nf_core/schema.py 77.24% <60.00%> (-0.56%) ⬇️
... and 18 more

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 e0d00d3...414cc9a. Read the comment docs.

Copy link
Contributor

@FriederikeHanssen FriederikeHanssen left a comment

Choose a reason for hiding this comment

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

Don't think I know enough python to approve. I tried syncing the testpipeline though, and that worked really well: nf-core/testpipeline#40
(Only fail, because I adapted the linting.yml to use the dev version. then it doesn't match the template and the linting rightfully complains)

@ewels
Copy link
Member Author

ewels commented May 13, 2022

Oh nice, thanks! I want to do a sync test where it automatically creates the PR for me still (it's that stuff with the GitHub API that I refactored) but a relief to know that the sync itself isn't playing up 😅

Copy link
Member

@drpatelh drpatelh left a comment

Choose a reason for hiding this comment

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

🚀 What could possibly go wrong! 😆

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.

🚀

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Member

@JoseEspinosa JoseEspinosa left a comment

Choose a reason for hiding this comment

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

Awesome job!
Though probably an opinion of a real pythonist would be more relevant 😛

.github/workflows/fix-linting.yml Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
@ewels ewels mentioned this pull request May 16, 2022
4 tasks
@ewels ewels mentioned this pull request May 16, 2022
4 tasks
@ewels ewels merged commit f8de281 into master May 16, 2022
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.