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

Create Marker Plots #61

Merged
merged 11 commits into from
Nov 6, 2023
Merged

Create Marker Plots #61

merged 11 commits into from
Nov 6, 2023

Conversation

rnmitchell
Copy link
Contributor

This PR will create a single PDF file containing the reads vs. CE allele for each locus.

Comment on lines +194 to +196
n = 100 if max_reads > 1000 else 10
max_yvalue = int(math.ceil(max_reads / n)) * n
increase_value = int(math.ceil((max_yvalue / 5)) / n) * n
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to make nice round values (10, 20, 100, 200, etc.) when determining the max y value of the dataset and what the incremental increase should be for the plots. The first line determines whether to round to 10s or 100s depending on the highest # of reads.

@rnmitchell
Copy link
Contributor Author

RebJ had requested the two separate pages: one showing the individual marker plots with its own y-axis scale (better for examining individual marker plots) and one showing the individual marker plots with the same y-axis scale (better for getting an idea of the profile as a whole).

@rnmitchell
Copy link
Contributor Author

This is ready for review @standage. I'm attaching two examples of the plots, one for the autosomal STRs and one for the sex chr STRs since you aren't able to see what they look like.
Uploading lusstr_output_Positive_Control_marker_plots.pdf…
Uploading lusstr_output_Positive_Control_sexchr_marker_plots.pdf…

@standage standage marked this pull request as ready for review November 3, 2023 18:44
Copy link
Member

@standage standage 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, just a couple of suggestions for your consideration.

README.md Outdated
@@ -165,7 +165,12 @@ Further, a second table (labeled as ```*_flanks.txt```) containing information r
* 3' Flanking Sequence Bracketed Notation
* Potential Issues (such as: Possible indel or partial sequence)

The ```Potential Issues``` column in this report is to draw attention to potential problem sequences (due to perhaps an indel or partial sequence) and requires the attention of the user to further evaluate the sequence for it's authenticity.
The ```Potential Issues``` column in this report is to draw attention to potential problem sequences (due to perhaps an indel or partial sequence) and requires the attention of the user to further evaluate the sequence for it's authenticity.
Copy link
Member

Choose a reason for hiding this comment

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

Typo: "for its authenticity".

Comment on lines 190 to 192
def make_plot(df, id, sex, sameyaxis):
sample_df = df[df["SampleID"] == id]
sample_id = f"{id}_sexchr" if sex else id
Copy link
Member

Choose a reason for hiding this comment

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

Suggestion for a marginal improvement in readability: make the sameyaxis argument a keyword argument by giving it a default value.

Comment on lines 184 to 187
make_plot(df, id, sex, False)
pdf.savefig()
make_plot(df, id, sex, True)
pdf.savefig()
Copy link
Member

Choose a reason for hiding this comment

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

That way, when someone is messing with the code 18 months from now and they're trying to remember what that dang 4th argument to the make_plot function is, passing the argument as a key/value pair will provide a great contextual reminder.

make_plot(df, id, sex, sameyaxis=False)

@@ -202,6 +250,7 @@ def main(input, out, kit, uas, sex, nocombine):
autosomal_final_table.to_csv(out, sep="\t", index=False)
else:
autosomal_final_table.to_csv(out, sep="\t", index=False)
marker_plots(autosomal_final_table, output_name, False)
Copy link
Member

Choose a reason for hiding this comment

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

Similar suggestion with the sex argument here.

marker_plots(autosomal_final_table, output_name, sex_chr=False)

@rnmitchell
Copy link
Contributor Author

Updated!

@standage standage merged commit 24fc3cb into master Nov 6, 2023
3 checks passed
@standage standage deleted the viz branch November 6, 2023 14:12
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