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

Tower UI no longer shows revisions for downloaded pipelines. #2308

Merged
merged 9 commits into from
Jun 12, 2023

Conversation

MatthiasZepper
Copy link
Member

Fix for issue #2307 and some other bugs discovered along the way:

  • nf-core download will now always create the latest branch to ensure that there is at least one branch in the repo. It will use the tag with the highest semantic version number. Otherwise, Tower's UI will fail to display the revisions.
  • A bug has been fixed that caused a TypeError in prompt_singularity_cachedir_remote(). In the documentation, I had missed that a validation str must be wrapped into a callable object. Now, SingularityCacheFilePathValidator in utils.py handles the validation of the interactive input to --singularity-cache-index.
  • In read_remote_containers(), I had missed that a call to prompt_singularity_cachedir_remote() still used the retry argument, even though I removed this argument from the final version of the function, because @mashehu had recommended a better way to implement the behaviour.
  • In find_container_images() I had denoted a logical and as & even though Python uses and.
  • In prompt_singularity_cachedir_remote() and summary_log, I have rephrased two messages to the user.

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated (Not required, since there had not been a release yet of the buggy code.)
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@MatthiasZepper MatthiasZepper changed the base branch from master to dev June 8, 2023 18:11
@nf-core nf-core deleted a comment from github-actions bot Jun 8, 2023
@nf-core nf-core deleted a comment from github-actions bot Jun 8, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 8, 2023

This PR is against the master 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 @MatthiasZepper,

It looks like this pull-request is has been made against the MatthiasZepper/tools master branch.
The master branch on nf-core repositories should always contain code from the latest release.
Because of this, PRs to master are only allowed if they come from the MatthiasZepper/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!

@codecov
Copy link

codecov bot commented Jun 8, 2023

Codecov Report

Merging #2308 (8abe1a6) into dev (711d074) will decrease coverage by 0.08%.
The diff coverage is 50.00%.

@@            Coverage Diff             @@
##              dev    #2308      +/-   ##
==========================================
- Coverage   72.97%   72.89%   -0.08%     
==========================================
  Files          78       78              
  Lines        8732     8756      +24     
==========================================
+ Hits         6372     6383      +11     
- Misses       2360     2373      +13     
Impacted Files Coverage Δ
nf_core/utils.py 81.40% <28.57%> (-0.66%) ⬇️
nf_core/download.py 61.68% <55.55%> (-0.24%) ⬇️

# "latest" is neither a branch (no heads exist) nor a tag, since is_valid_object() returned False.
# try to determine highest contained version number from desired revisions, to which latest will be pinned.
try:
latest = sorted(desired_revisions, key=StrictVersion, reverse=True)[0]
Copy link
Contributor

Choose a reason for hiding this comment

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

why don't you use looseversion here?

Copy link
Member Author

@MatthiasZepper MatthiasZepper Jun 9, 2023

Choose a reason for hiding this comment

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

distutils module is unfortunately deprecated. Will therefore rewrite and use neither StrictVersion nor LooseVersion. Took a simplified Regex derived from the one in PEP386.

nf_core/download.py Outdated Show resolved Hide resolved
nf_core/download.py Outdated Show resolved Hide resolved
nf_core/download.py Outdated Show resolved Hide resolved
@MatthiasZepper MatthiasZepper force-pushed the Issue_2307 branch 2 times, most recently from 3f8270a to d84a43b Compare June 9, 2023 16:32
…odule is deprecated. See PEP 632 and PEP 386 for details.
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.

2 participants