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
Make FASTA Index methods and structs public #421
Comments
I did a quick For the respective section, I saw contributions by @johanneskoester, @HenningTimm and @MikkelSchubert. Maybe one of them has any indicators about them being private---whether that was intentional or not? |
I vaguely remember working on that ;) IIRC I wanted to preserve the existing visibility structure since I was relatively new to Rust and didn't want to make stupid mistakes. I also remember that making it public would have my changes back than easier. I would not be opposed making it public. |
I guess what I was trying to achieve when I raised this was a way of getting the length of a contig in an index - if it is in the index. My current solution is a bit more convoluted than is necessary given this information is accessible by some private functions/structs. Something which makes me slightly hesitant to just make the private stuff public is the method I mentioned in the beginning, Ok, here is an example. We have an index
With the current, public-facing API, if I want to check for the presence of a contig in this index, and get its length if it is present, I have to do something like this let faidx = fasta::IndexedReader::from_file(file);
let contig = "CHROMOSOME_I";
let mut contig_len: Option<usize> = None;
for seq in faidx.index.sequences() {
if seq.name == contig {
contig_len = Some(seq.len);
break;
}
}
match contig_len {
// do something with the Some value...
} This seems like a lot of work for a fairly basic requirement of an index I think? So, if we made let contig_len = match faidx.idx(contig) {
Ok(idx_record) => Some(idx_record.len),
Err(_) => None Another alternative would be to leave the visibility as is and implement a method, either on // implemented on IndexedReader
let contig_len: Option<usize> = faidx.length_of(contig);
// implemented on Index
let contig_len: Option<usize> = faidx.index.length_of(contig); I guess one benefit to make the Apologies for the waffling, but it seems this is an important design decision |
I'm not sure if it is intentional or not, but
fasta::IndexRecord
is currently private. Access to this struct would make working with FASTA indices much nicer. Additionally, there are some methods, such asidx
which are also private.Are there any methods/structs where a conscious decision was made to keep them private?
The text was updated successfully, but these errors were encountered: