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

Updating tools/bowtie2 from version 2.5.3 to 2.5.4 #6029

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

gxydevbot
Copy link
Collaborator

Hello! This is an automated update of the following tool: tools/bowtie2. I created this PR because I think the tool's main dependency is out of date, i.e. there is a newer version available through conda.

I have updated tools/bowtie2 from version 2.5.3 to 2.5.4.

Project home page: http://bowtie-bio.sourceforge.net/bowtie2

For any comments, queries or criticism about the bot, not related to the tool being updated in this PR, please create an issue here.

@bgruening
Copy link
Member

 .. WARNING (ConditionalParamTypeBool): Conditional [read_group_id_conditional] first param of type="boolean" is discouraged, use a select
.. WARNING (ConditionalParamTypeBool): Conditional [read_group_sm_conditional] first param of type="boolean" is discouraged, use a select
.. WARNING (ConditionalParamTypeBool): Conditional [read_group_lb_conditional] first param of type="boolean" is discouraged, use a select
.. WARNING (ConditionalParamTypeBool): Conditional [read_group_id_conditional] first param of type="boolean" is discouraged, use a select
.. WARNING (ConditionalParamTypeBool): Conditional [read_group_sm_conditional] first param of type="boolean" is discouraged, use a select
.. WARNING (ConditionalParamTypeBool): Conditional [read_group_lb_conditional] first param of type="boolean" is discouraged, use a select

Thats the shared macro. @bernt-matthias do you remember how we fixed this the last time without touching all tools using this macro?

@mvdbeek
Copy link
Member

mvdbeek commented Aug 22, 2024

I would very strongly suggest to ignore this and maybe decrease the log level to info. If you make the switch at a minimum the tool state in all workflows using this tool will be invalid on the next update, but also lot's of cheeath got broken by this change since you don't actually have boolean values anymore in a select. It's also easy to fix on the UI side: If we have a boolean in a conditional, render it as a select (this was the reason for that recommendation).

@bernt-matthias
Copy link
Contributor

We can either add ConditionalParamTypeBool as linter exception (i.e. merge this #6184). Or decrease the linter level in order to have this for all repos without any additional efforts.

What would be your preference @mvdbeek ?

@mvdbeek
Copy link
Member

mvdbeek commented Sep 2, 2024

Or decrease the linter level in order to have this for all repos without any additional efforts.

This, or just remove it completely. It's not even an info thing IMO.

@bgruening
Copy link
Member

Its a IUC recommendation and we are here in the IUC repo. I think a stringent UI and a good UX has some big benefits.

If we can merge those tools on exceptions and they get still deployed that is fine, isn't it? Why would we remove this if we want to encourage our recommendations?

I understand that for broadly used tools big parameter changes have a large downstream effect. And in this case here its probably not worthwhile.
In general, I'm no in favour of abandon our best practices just because a tool is frequently used and we are afraid of upstream work.

@bernt-matthias
Copy link
Contributor

I agree with @mvdbeek .. we should

@gxydevbot
Copy link
Collaborator Author

There are new updates, they have been integrated to the PR, check the file diff.

@gxydevbot
Copy link
Collaborator Author

There are new updates, they have been integrated to the PR, check the file diff.

@gxydevbot
Copy link
Collaborator Author

There are new updates, they have been integrated to the PR, check the file diff.

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.

4 participants