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

fix: fixed include flag KeyError #452

Merged
merged 3 commits into from
Jan 20, 2022
Merged

fix: fixed include flag KeyError #452

merged 3 commits into from
Jan 20, 2022

Conversation

AKBrueggemann
Copy link
Collaborator

Description

There was a problem with the include_flag in common.smk. The get_assemblies_for_submission function gives the path to the faster file, not the name, leading to a KeyError in get_include_flag. This causes the inclusion of files in the high-quality-genomes, that should be excluded. Problem fixed by changing the functions used in get_include_flag_for_date and taking into account the lengths of the lists from get_assemblies_for_submission and get_sample_for_date.

Related Issue

Checklist

Copy link
Contributor

@thomasbtf thomasbtf left a comment

Choose a reason for hiding this comment

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

Nice catch! Just one remark

Comment on lines 1289 to 1290
else:
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

I`m not sure if this is needed 🤔

Comment on lines 1289 to 1290
else:
continue
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
else:
continue

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd also vote for leaving it out. If it's needed, we will see it 😉

Comment on lines 1289 to 1290
else:
continue
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd also vote for leaving it out. If it's needed, we will see it 😉

@AKBrueggemann AKBrueggemann merged commit f47dad2 into master Jan 20, 2022
@AKBrueggemann AKBrueggemann deleted the include-flag-fix branch January 20, 2022 08:27
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.

3 participants