+
Skip to content

Ftr/test datasets easy copy #3720

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Aug 27, 2025
Merged

Ftr/test datasets easy copy #3720

merged 6 commits into from
Aug 27, 2025

Conversation

JulianFlesch
Copy link
Contributor

Addresses issue 3718 regarding plain text output from test-datasets subcommand.

Also adds a warning when using -p and -u at the same time and informs about this behavior in help text.

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

@github-actions GitHub Actions
Copy link
Contributor

This PR is against the main branch ❌

  • Do not close this PR
  • Click Edit and change the base to dev
  • This CI test will remain failed until you push a new commit

Hi @JulianFlesch,

It looks like this pull-request is has been made against the JulianFlesch/nf-core-tools main branch.
The main branch on nf-core repositories should always contain code from the latest release.
Because of this, PRs to main are only allowed if they come from the JulianFlesch/nf-core-tools dev branch.

You do not need to close this PR, you can change the target branch to dev by clicking the "Edit" button at the top of this page.
Note that even after this, the test will continue to show as failing until you push a new commit.

Thanks again for your contribution!

@JulianFlesch JulianFlesch changed the base branch from main to dev August 22, 2025 10:56
@JulianFlesch JulianFlesch requested review from ewels and jfy133 August 22, 2025 10:56
@JulianFlesch JulianFlesch self-assigned this Aug 22, 2025
@codecov Codecov
Copy link

codecov bot commented Aug 22, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 37.50000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 77.34%. Comparing base (283e8c5) to head (7ed718d).
⚠️ Report is 25 commits behind head on dev.

Files with missing lines Patch % Lines
nf_core/test_datasets/list.py 25.00% 9 Missing ⚠️
nf_core/test_datasets/search.py 75.00% 1 Missing ⚠️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jfy133
jfy133 approved these changes Aug 22, 2025
View reviewed changes
Copy link
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

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

Is the 'fix' itself line 74 of list.py? If so looks simple enough but I will leave it to actual python people to confirm

@JulianFlesch
Copy link
Contributor Author

Is the 'fix' itself line 74 of list.py?

Yep, this commit

The rest just adds some info text and warnings to notify users that they won't see urls and nextflow import paths at the same time and -p / --generate-nf-path takes precedence.

mirpedrol
mirpedrol reviewed Aug 22, 2025
View reviewed changes
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.

Changes look good, but there currently no way to make plain_text_output true, so I think we should remove this option

@jfy133
Copy link
Member

jfy133 commented Aug 23, 2025

Changes look good, but there currently no way to make plain_text_output true, so I think we should remove this option

So there is no way to actually fix the bug?! 😱

@mirpedrol
Copy link
Member

So there is no way to actually fix the bug?!

Yes it is fixed! Sorry maybe I didn't explain well. With this PR, when generating a URL or a path, the output is printed without a table. My only comment was that the option plain_text_output is not used anywhere, so we can remove this bit of the code.

@jfy133
Copy link
Member

jfy133 commented Aug 25, 2025

So there is no way to actually fix the bug?!

Yes it is fixed! Sorry maybe I didn't explain well. With this PR, when generating a URL or a path, the output is printed without a table. My only comment was that the option plain_text_output is not used anywhere, so we can remove this bit of the code.

Ahh phew! Ok sorry 😅

I was worried tools had become to rich and and it cannot go back 😬

@JulianFlesch JulianFlesch merged commit 76b5635 into nf-core:dev Aug 27, 2025
102 checks passed
@JulianFlesch JulianFlesch mentioned this pull request Aug 28, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mirpedrol mirpedrol mirpedrol left review comments

@jfy133 jfy133 jfy133 approved these changes

@ewels ewels Awaiting requested review from ewels

Assignees

@JulianFlesch JulianFlesch

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@JulianFlesch @jfy133 @mirpedrol
点击 这是indexloc提供的php浏览器服务，不要输入任何密码和下载