Bc generalize attach barcodes

[benjamincarlin]

Purpose

Please explain the purpose of this PR and include links to any GitHub issues that it fixes:

• No issue is linked to this PR.

---

Changes

Please list out what major changes were made in this PR to address the issue:

• No changes.

---

Review Instructions

Please provide instructions about how should a reviewer test/verify the changes in this PR:

• No instructions.

---

PR Checklist

Please ensure the following when opening a PR:

• This PR added or updated tests.
• This PR updated docstrings or documentation.
• This PR applied Python style guidelines, specifically followed NumPy style docstrings for this repo.
• This PR considered generalizability beyond our own use case.

---

Follow-up Discussions

Please append follow-up discussions and issues during the review process below:

Possible Tests:

• test the possible barcodes combinations of cell barcode, molecule barcode and sample barcode:
  a test with only a cell barcode, a test with cell and sample barcodes, a test with cell and molecule barcodes, a test with cell, molecule and sample barcodes, a test with only a sample barcode, a test with a sample and molecule barcodes and a test with only molecule barcodes

• test the possible barcode combinations that have a cell barcode with and without a whitelist

• test invalid start positions for cell, molecule and sample barcodes

• test invalid lengths for cell, molecule and sample barcodes

• test that cell and molecule barcodes do not overlap

• test that barcode start positions must be specified with barcode lengths

• test that an index fastq must be provided if a sample barcode start position and length are given

[rexwangcc]

Thanks for the work, I left some comments. Also once @barkasn take a look at this, let's bring Ambrose to the loop to get his insights!

[codecov-io]

Codecov Report

Merging #57 into master will decrease coverage by 2.19%.
The diff coverage is 20.27%.

@@           Coverage Diff            @@
##           master     #57     +/-   ##
========================================
- Coverage   96.09%   93.9%   -2.2%     
========================================
  Files          27      27             
  Lines        2486    2560     +74     
========================================
+ Hits         2389    2404     +15     
- Misses         97     156     +59

Impacted Files	Coverage Δ
src/sctools/platform.py	67.18% <20.27%> (-18.77%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3b1199d...4be3e1b. Read the comment docs.

[rexwangcc]

👍 LGTM

[ambrosejcarr]

Hi Ben!

Thanks for the PR! This is a great start. I've made some suggestions, and definitely agree that adding some tests that either (1) synthesize test data (2) work from (small) checked in examples or (3) pull from a public bucket, would be helpful.

[benjamincarlin]

@ambrosejcarr Thanks for your review! I left two responses to your comments. Please let me know if there is anything else I can do!

[ambrosejcarr]

Hi @benjamincarlin

Purpose

Please explain the purpose of this PR and include links to any GitHub issues that it fixes:

This section is the place to put the rationale behind the PR. I apologize -- I misinterpreted the purpose of your changes and based them on our previous conversations about a specific assay.

Are you hoping that this would provide a general solution across assays? If so, it would be a good idea to write down the assumptions the tool is making. For example, that the sample barcode would always be found on the i7 file, that the cell and molecule barcodes would be found on r1, and that no barcode is contained on u2.

To make this a bit more general, I might omit the numerical assignments to the reads. r1 and i1 could become a list of one or more input fastq files, and the tool could enable a user to specify which fastq file the tag should be associated with. I make these suggestions from past experience with In-drop and Mars-seq, where the barcodes are not always in the same positions or on the same fastq files.

I think these changes, combined with what you've already written, would do a great job of making this more general. We could talk later to @rexwangcc or @samanehsan about deprecating the 10x-specific tool in our workflows.

Do these suggestions make sense to you?

[benjamincarlin]

Hi @ambrosejcarr

I provided a purpose and description to both the class and input file arguments. In addition, I believe that r1 and i1 becoming a list of one or more input fastq's, is expanding the scope of this PR. In order to unblock my workflow would you be willing to create an issue linking back this PR instead?

[ambrosejcarr]

@benjamincarlin pushing the additional scope into an issue sounds fine to me. Would you mind making that issue? I've made one suggestion above but otherwise this looks good to merge. Thanks for the contribution!