Skip to content

Commit

Permalink
Remove fetchSizeLimit to complement the removal of chunkSizeLimit
Browse files Browse the repository at this point in the history
  • Loading branch information
cmdcolin committed Jun 13, 2024
1 parent d9e5c06 commit 461cfec
Show file tree
Hide file tree
Showing 3 changed files with 582 additions and 673 deletions.
4 changes: 0 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -344,10 +344,6 @@ that show insertions, deletions, substitutions, etc.
- `args.cacheSize`
**[number](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Number)?**
optional maximum number of CRAM records to cache. default 20,000
- `args.fetchSizeLimit`
**[number](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Number)?**
optional maximum number of bytes to fetch in a single getRecordsForRange
call. Default 3 MiB.
- `args.checkSequenceMD5`
**[boolean](https://developer.mozilla.org/docs/Web/JavaScript/Reference/Global_Objects/Boolean)?**
default true. if false, disables verifying the MD5 checksum of the reference
Expand Down
24 changes: 2 additions & 22 deletions src/indexedCramFile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,19 @@ export interface CramIndexLike {
export default class IndexedCramFile {
public cram: CramFile
public index: CramIndexLike
private fetchSizeLimit: number

/**
*
* @param {object} args
* @param {CramFile} args.cram
* @param {Index-like} args.index object that supports getEntriesForRange(seqId,start,end) -> Promise[Array[index entries]]
* @param {number} [args.cacheSize] optional maximum number of CRAM records to cache. default 20,000
* @param {number} [args.fetchSizeLimit] optional maximum number of bytes to fetch in a single getRecordsForRange call. Default 3 MiB.
* @param {boolean} [args.checkSequenceMD5] - default true. if false, disables verifying the MD5
* checksum of the reference sequence underlying a slice. In some applications, this check can cause an inconvenient amount (many megabases) of sequences to be fetched.
*/
constructor(
args: {
index: CramIndexLike
fetchSizeLimit?: number
} & (
| { cram: CramFile }
| ({
Expand Down Expand Up @@ -70,8 +67,6 @@ export default class IndexedCramFile {
if (!this.index.getEntriesForRange) {
throw new Error('invalid arguments: not an index')
}

this.fetchSizeLimit = args.fetchSizeLimit || 3000000
}

/**
Expand Down Expand Up @@ -104,11 +99,6 @@ export default class IndexedCramFile {
const seqId = seq
const slices = await this.index.getEntriesForRange(seqId, start, end)
const totalSize = slices.map(s => s.sliceBytes).reduce((a, b) => a + b, 0)

Check warning on line 101 in src/indexedCramFile.ts

View workflow job for this annotation

GitHub Actions / Lint, build, and test on node 20.x and ubuntu-latest

'totalSize' is assigned a value but never used
if (totalSize > this.fetchSizeLimit) {
throw new CramSizeLimitError(
`data size of ${totalSize.toLocaleString()} bytes exceeded fetch size limit of ${this.fetchSizeLimit.toLocaleString()} bytes`,
)
}

// TODO: do we need to merge or de-duplicate the blocks?

Expand Down Expand Up @@ -180,17 +170,7 @@ export default class IndexedCramFile {

const mateRecordPromises = []
const mateFeatPromises: Promise<CramRecord[]>[] = []

const mateTotalSize = mateChunks
.map(s => s.sliceBytes)
.reduce((a, b) => a + b, 0)
if (mateTotalSize > this.fetchSizeLimit) {
throw new Error(
`mate data size of ${mateTotalSize.toLocaleString()} bytes exceeded fetch size limit of ${this.fetchSizeLimit.toLocaleString()} bytes`,
)
}

mateChunks.forEach(c => {
for (const c of mateChunks) {
let recordPromise = this.cram.featureCache.get(c.toString())
if (!recordPromise) {
recordPromise = this.getRecordsInSlice(c, () => true)
Expand All @@ -210,7 +190,7 @@ export default class IndexedCramFile {
return mateRecs
})
mateFeatPromises.push(featPromise)
})
}
const newMateFeats = await Promise.all(mateFeatPromises)
if (newMateFeats.length) {
const newMates = newMateFeats.reduce((result, current) =>
Expand Down
Loading

0 comments on commit 461cfec

Please sign in to comment.