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

if flowcell_lanes is a column in sample metadata, forces it to be a string… #100

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

uwidocki
Copy link
Contributor

…otherwise, the collating function will not work if it is a single integer. Minor edits to make defaults in the script match new defaults we also use on Jenkins UI

…e collating function will not work if it is a single integer. Minor edits to make defaults in the script match new defaults we also use on Jenkins UI
Copy link
Contributor

@jdavis3141 jdavis3141 left a comment

Choose a reason for hiding this comment

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

Is there the potential that this would cause an issue when merging with our uncollapsed_raw_counts since we're not enforcing the same type there?

@uwidocki
Copy link
Contributor Author

Is there the potential that this would cause an issue when merging with our uncollapsed_raw_counts since we're not enforcing the same type there?

There is no conflict with this merge since the lanes in uncollapsed were always read in as ints because it is a single integer in each entry, and the way flowcell_lanes in sample_meta is handled seems to assume they are always strings since the list of lanes is read in as a string.

@jdavis3141
Copy link
Contributor

Thanks! Looks good to me then!

Copy link
Collaborator

@ashishge1 ashishge1 left a comment

Choose a reason for hiding this comment

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

Would be good to check if QC_images.R or any of the other modules using sample_meta needs a similar fix. Otherwise, looks good to me!

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