Skip to content

Pytest requirements#1620

Merged
ewels merged 5 commits into
nf-core:devfrom
fabianegli:pytest-requirements
Jun 2, 2022
Merged

Pytest requirements#1620
ewels merged 5 commits into
nf-core:devfrom
fabianegli:pytest-requirements

Conversation

@fabianegli

@fabianegli fabianegli commented Jun 1, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

nf-core has specific pytest and pytest-workflow version dependencies. I added them to the requirements.txt file.

The pytest dependency has been moved from the dev to the standard requirements file because it is a requirement since the introduction of the nf-core modules test command.

Surfaced in nf-core Slack

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

codecov Bot commented Jun 1, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #1620 (2117653) into dev (dc4e8f1) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev    #1620   +/-   ##
=======================================
  Coverage   64.71%   64.71%           
=======================================
  Files          54       54           
  Lines        6235     6235           
=======================================
  Hits         4035     4035           
  Misses       2200     2200

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 dc4e8f1...2117653. Read the comment docs.

@jfy133 jfy133 requested review from ewels and mirpedrol June 2, 2022 07:41
@jfy133

jfy133 commented Jun 2, 2022

Copy link
Copy Markdown
Member

Would this require a separate bioconda update? Or is the recipe updated automated from requirements.txt?

ewels
ewels approved these changes Jun 2, 2022
View reviewed changes

@ewels ewels left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thanks!

yes this also needs to be done separately in bioconda. Can edit the auto-generated PR that happens after release on PyPI or can update the recipe right away whilst incrementing the build number.

I think the latter approach would be best here 👍🏻

@ewels ewels merged commit f476a07 into nf-core:dev Jun 2, 2022
@fabianegli fabianegli deleted the pytest-requirements branch June 2, 2022 08:56
@fabianegli

fabianegli commented Jun 3, 2022

Copy link
Copy Markdown
Contributor Author

My pleasure 😄

@ewels

ewels commented Jun 3, 2022

Copy link
Copy Markdown
Member

@fabianegli did you do the Bioconda PR as well?

@fabianegli

fabianegli commented Jun 6, 2022

Copy link
Copy Markdown
Contributor Author

No, I haven't. Which repo? I didn't find nf-core in the recipies in the bioconda repo.

@ewels

ewels commented Jun 6, 2022

Copy link
Copy Markdown
Member

Here: https://github.com/bioconda/bioconda-recipes/tree/master/recipes/nf-core

Need to bump the build number to 1 and then add to the requirements list.

The docs say you should test locally and stuff but I wouldn't bother personally. Just fork, edit and push a PR and see if the tests pass.. :)

@fabianegli

fabianegli commented Jun 6, 2022

Copy link
Copy Markdown
Contributor Author

bioconda/bioconda-recipes#35232

@fabianegli fabianegli mentioned this pull request Jun 6, 2022
Merged
@ewels

ewels commented Jun 6, 2022

Copy link
Copy Markdown
Member

Nice! And because you made the PR, I was able to merge it 😄

@fabianegli

fabianegli commented Jun 7, 2022

Copy link
Copy Markdown
Contributor Author

In bioconda, something seems to be off, though. The CI for the nightly fails: https://github.com/bioconda/bioconda-recipes/runs/6766508609

Someone reported the issue: bioconda/bioconda-recipes#35015

Not sure if this is something that needs our attention, but I think it means the new recipe has not been published, yet? At least when I installed nf-core from bioconda just now nf-core build 0 was installed.

@ewels

ewels commented Jun 12, 2022

Copy link
Copy Markdown
Member

Hmm, not ideal. Maybe worth posting in the Bioconda Gitter chat..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ewels ewels ewels approved these changes

@mirpedrol mirpedrol Awaiting requested review from mirpedrol

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@fabianegli @jfy133 @ewels