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

Release/1.2.1 #61

Merged
merged 22 commits into from
Apr 7, 2020
Merged

Release/1.2.1 #61

merged 22 commits into from
Apr 7, 2020

Conversation

jjzieve
Copy link
Contributor

@jjzieve jjzieve commented Mar 28, 2020

No description provided.

jjzieve and others added 21 commits June 6, 2019 19:49
Merge pull request #44 from Illumina/release/1.2.0
Addresses case where ALT allele(s) are one base, hence an indel and n…
…D/I]) but consistent genotypes (e.g 1 vs. 3)
…-for-indels

Handles case where indels with opposite orientation (e.g. [I/D] vs. […
Applies change from BeadArrayFiles repository
@jjzieve jjzieve mentioned this pull request Mar 28, 2020
Copy link

@CodeWithCare CodeWithCare left a comment

Choose a reason for hiding this comment

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

Looks good. Some minor questions.

@@ -55,6 +56,8 @@ def generate_sample_format_info(self, bpm_records, vcf_record, sample_name):
sample_name (string): Name of sample

Returns:
float: Minimum GenCall score (or None for empty arguments)
int: Minimum GenCall score, encoded as a phred quality (or None for empty arguments)

Choose a reason for hiding this comment

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

You will know better but isn't phred score a float?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, its an int

"""
return self._get_min_gencall_score(bpm_records)
min_gencall_score = self._get_min_gencall_score(bpm_records)
phred_quality_score = int(round(-10*log10(min(1.0, max(0.00001, 1 - min_gencall_score)))))

Choose a reason for hiding this comment

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

nvm I see the conversion here but not sure why an int conversion is required?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you're right I don't think it needs it, will fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nvm apparently it does. Prints one decimal place even though the type should be an int, not entirely sure without digging deeper but it passes the regression tests the way it is

@@ -758,6 +758,8 @@ def __parse_file(self, manifest_file):
all_norm_ids = set()
for locus_idx in range(self.num_loci):
self.normalization_ids[locus_idx] += 100 * self.assay_types[locus_idx]
# To mimic the byte-wrapping behavior from GenomeStudio, AutoCall, IAAP take the mod of 256
self.normalization_ids[locus_idx] %= 256

Choose a reason for hiding this comment

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

Nice. Finally lols.

@jjzieve jjzieve merged commit 3fdd391 into master Apr 7, 2020
@jjzieve jjzieve deleted the release/1.2.1 branch April 7, 2020 18:56
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