search

GMOD / cram-js

Read CRAM v3 and v2 in node or in the browser
MIT License
18 stars 9 forks source link

Remove dependency on long, abortable-promise-cache #128

Closed cmdcolin closed 2 months ago

cmdcolin commented 1 year ago

This removes some dependencies on some packages

The removal of abortable-promise-cache could change aborting behavior in a slight way: if multiple clients try to parseIndex, and one aborts, then all will abort

I don't believe this will affect jbrowse code, and would probably not affect other consumers

This also removes the long package. My testing seemed to conclude that type 'I' in the data records are Uint32 instead of Uint64 requiring the Long package. I couldn't find exact reference to this in the CRAMv3.1 pdf but console logging the buffer showed only 4 bytes instead of 8, and that also corresponds to type i/I in bam-js being 4 byte ints (https://github.com/GMOD/bam-js/blob/507c7284749603862ef72a473db6435c8a865102/src/record.ts#L211-L220)

The typescript in this zone of the code is slightly improved also, but may benefit from some further work (there is technically some usage of Uint8Array|number but it ignores the number usage right now)

codecov[bot] commented 1 year ago

Codecov Report

Attention: Patch coverage is 96.55172% with 2 lines in your changes are missing coverage. Please review.

Project coverage is 87.36%. Comparing base (8db17b9) to head (9cad54f).

:exclamation: Current head 9cad54f differs from pull request most recent head f22e073. Consider uploading reports for the commit f22e073 to get more accurate results

Files Patch % Lines
src/craiIndex.ts 95.12% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #128 +/- ## ========================================== - Coverage 90.26% 87.36% -2.91% ========================================== Files 38 40 +2 Lines 1952 2018 +66 Branches 400 403 +3 ========================================== + Hits 1762 1763 +1 - Misses 176 249 +73 + Partials 14 6 -8 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

rbuels commented 1 year ago

This is fine, though I'm a little worried about the aborting behavior. In JBrowse, if you opened a track, and the .crai download was being slow for whatever reason, and the user scrolls around a bit, I think this could well lead to the track just breaking and needing to be reloaded.