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

Refactor fasta reader index usage and allow filelike object use #51

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

GarrettNg
Copy link
Contributor

@GarrettNg GarrettNg commented Nov 3, 2023

The fasta implementation was a little tricky since there are multiple relevant readers upstream in noodles: fasta::Reader, fasta::IndexedReader, and fasta::fai::Reader. The core records_to_ipc function relies on the query method supplied by the fasta::Reader, so the fasta::Reader was turned into a FastaReader struct field. Since the query method also relies on the fasta index, index was made into a FastaReader struct field and is now read through a separate fasta::fai::Reader instead of using the fasta::IndexedReader, which has access to the index, but lacks the query method. (EDIT: yes it does have a query method)

Filelike object compatability was also implemented in line with the other readers, and generics have been sprinkled about accordingly.

Also now provides a proper error message when a .fai index file can't be found instead of providing a generic "file not found" error which a user may attribute to the fasta file and not the index file.

@GarrettNg GarrettNg requested review from nvictus and manzt November 3, 2023 21:46
@GarrettNg GarrettNg changed the title Refactor fasta reader index usage and allow filelike object use Refactor fastq reader index usage and allow filelike object use Nov 13, 2023
@GarrettNg GarrettNg changed the title Refactor fastq reader index usage and allow filelike object use Refactor fasta reader index usage and allow filelike object use Nov 13, 2023
@GarrettNg
Copy link
Contributor Author

I'm going to revisit to make it use the IndexedReader when possible. Also, the IndexedReader does have a query method.

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.

1 participant