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 extra postfix in sample names for BCL Convert #440

Merged
merged 5 commits into from
Nov 6, 2024

Conversation

chuan-wang
Copy link

Validation 24_20.

Copy link

codecov bot commented Oct 31, 2024

Codecov Report

Attention: Patch coverage is 0% with 8 lines in your changes missing coverage. Please review.

Project coverage is 29.00%. Comparing base (da60ac2) to head (b3d8446).
Report is 8 commits behind head on master.

Files with missing lines Patch % Lines
taca/illumina/Runs.py 0.00% 8 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #440      +/-   ##
==========================================
- Coverage   29.05%   29.00%   -0.05%     
==========================================
  Files          31       31              
  Lines        4898     4906       +8     
==========================================
  Hits         1423     1423              
- Misses       3475     3483       +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@alneberg alneberg requested a review from aanil November 1, 2024 08:51
# Remove the trailing "_SX" postfix from samples names for BCL Convert when it handles SmartSeq3 libraries
for entry in html_report_laneBarcode_parser.sample_data:
if "_S" in entry["Sample"]:
entry["Sample"] = "_".join(entry["Sample"].split("_")[:2])
Copy link
Member

Choose a reason for hiding this comment

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

If we are removing only the last element wouldn't "_".join(entry["Sample"].split("_")[:-1]) be better?

Copy link
Author

Choose a reason for hiding this comment

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

"".join(entry["Sample"].split("")[:-1])

Fixed

html_report_laneBarcode_parser.sample_data,
key=lambda k: (k["Lane"].lower(), k["Sample"]),
)
_generate_lane_html(
Copy link
Member

Choose a reason for hiding this comment

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

I'm not entirely sure how this works, but are we overwriting the file that exists at dest_html_report_laneBarcode here?

Copy link
Member

Choose a reason for hiding this comment

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

Or is it the same file here since we symlink it os.symlink(source, dest)?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, we are overwriting the file. Otherwise we will have to create hard links of all files under source to dest individually, in which case the scripts will be much more complicated...

Copy link
Member

@aanil aanil left a comment

Choose a reason for hiding this comment

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

👍

@chuan-wang chuan-wang merged commit 4bd531a into NationalGenomicsInfrastructure:master Nov 6, 2024
7 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants