clintval / sample-sheet

Parse Illumina sample sheets with Python
https://sample-sheet.rtfd.io
MIT License
49 stars 15 forks source link

CellRanger indexes are not recognized #92

Closed golharam closed 4 years ago

golharam commented 4 years ago

Is it possible to bypass index validation?

    sample_sheet = SampleSheet(args.samplesheet)
  File "/home/ec2-user/cellranger-docker/env/lib/python3.6/site-packages/sample_sheet/__init__.py", line 418, in __init__
    self._parse(self.path)
  File "/home/ec2-user/cellranger-docker/env/lib/python3.6/site-packages/sample_sheet/__init__.py", line 524, in _parse
    self.add_sample(Sample(dict(zip(sample_header, line))))
  File "/home/ec2-user/cellranger-docker/env/lib/python3.6/site-packages/sample_sheet/__init__.py", line 296, in __init__
    raise ValueError(f'Not a valid index: {value}')
ValueError: Not a valid index: SI-GA-F8
clintval commented 4 years ago

Looks reasonable to me! We could update what is considered a valid index to include the union of characters Illumina and 10x allow here:

https://github.com/clintval/sample-sheet/blob/8da5c6b5ef69170c456b6e7b7673fc2d57536fa2/sample_sheet/__init__.py#L275

I probably won't have time to get to in the next few days FYI but it's now on my radar.

golharam commented 4 years ago

I just submitted a PR with the proposed change.

On Wed, Dec 4, 2019 at 4:13 PM Clint Valentine notifications@github.com wrote:

Looks reasonable to me! We could update what is considered a valid index to include the union of characters Illumina and 10x allow here:

https://github.com/clintval/sample-sheet/blob/8da5c6b5ef69170c456b6e7b7673fc2d57536fa2/sample_sheet/__init__.py#L275

I probably won't have time to get to in the next few days FYI but it's now on my radar.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/clintval/sample-sheet/issues/92?email_source=notifications&email_token=AAFD2Z3Y4UHJ25F763PLJWDQXAMRNA5CNFSM4JVMN6FKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEF6QJAY#issuecomment-561841283, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAFD2Z52ZYKFIUZ4EAMSWN3QXAMRNANCNFSM4JVMN6FA .

clintval commented 4 years ago

Thank you for the patch, and the unit test! I just published sample_sheet==0.10.0 with this feature. Please pip install! Bioconda will find the change automatically in the next few days.

clintval commented 4 years ago

I'm migrating to an official public API freeze in 1.0.0. The API may change slightly, but function should remain the same.

Any feature requests, critiques of the API, and criticisms welcome before I commit. Thanks!

golharam commented 4 years ago

I just pushed a final change as I missed an index set in the first PR. I wonder if an option to bypass index checking would be good for a future release.

clintval commented 4 years ago

Thanks for the idea! Noted!

clintval commented 4 years ago

Just published 0.11.0 to PyPi, Bioconda should follow after some period of lag.