Conversation
TheJeran
requested changes
Feb 10, 2026
src/netcdf-getters.ts
Outdated
| else arrayData = module.nc_get_vara_double(workingNcid, varid, start, count); | ||
|
|
||
| if (arrayData.result !== NC_CONSTANTS.NC_NOERR && arrayData.result !== NC_CONSTANTS.NC_ERANGE) { | ||
| throw new Error(`nc_get_vara failed with error code: ${arrayData.result}`); |
Collaborator
There was a problem hiding this comment.
This is the line that tells you the error. It's in the console though
| -DHDF5_ENABLE_TESTS=OFF \ | ||
| -DCMAKE_C_BYTE_ORDER=LITTLE_ENDIAN \ | ||
| -DHDF5_DISABLE_COMPILER_WARNINGS=ON \ | ||
| -DHDF5_ENABLE_Z_LIB_SUPPORT=ON \ |
| -DBUILD_UTILITIES=OFF \ | ||
| -DBUILD_TESTING=OFF \ | ||
| -DENABLE_TESTS=OFF \ | ||
| -DENABLE_FILTER_DEFLATE=ON \ |
| -DBUILD_TESTING=OFF \ | ||
| -DENABLE_TESTS=OFF \ | ||
| -DENABLE_FILTER_DEFLATE=ON \ | ||
| -DENABLE_ZLIB=ON \ |
| -DENABLE_TESTS=OFF \ | ||
| -DENABLE_FILTER_DEFLATE=ON \ | ||
| -DENABLE_ZLIB=ON \ | ||
| -DCMAKE_C_BYTE_ORDER=LITTLE_ENDIAN \ |
Collaborator
There was a problem hiding this comment.
This is redundant. IS the default setting
Comment on lines
+481
to
+506
| if (arrayType === 2) { | ||
| const textData = module.nc_get_vara_text(workingNcid, varid, start, count); | ||
| if (textData.result !== NC_CONSTANTS.NC_NOERR && textData.result !== NC_CONSTANTS.NC_ERANGE) { | ||
| throw new Error(`nc_get_vara_text failed with error code: ${textData.result}`); | ||
| } | ||
| if (!textData.data) { | ||
| throw new Error("nc_get_vara_text returned no data"); | ||
| } | ||
| const chars = textData.data; | ||
| // For slices, the last dimension of 'count' is the string length | ||
| if (count.length > 0) { | ||
| const strLen = count[count.length - 1]; | ||
| if (strLen === 0) return []; | ||
| const numStrings = chars.length / strLen; | ||
| const strings: string[] = []; | ||
| const decoder = new TextDecoder(); | ||
|
|
||
| for (let i = 0; i < numStrings; i++) { | ||
| const strStart = i * strLen; | ||
| const strEnd = strStart + strLen; | ||
| strings.push(decoder.decode(chars.subarray(strStart, strEnd)).replace(/\0/g, '')); | ||
| } | ||
| return strings; | ||
| } | ||
| return [new TextDecoder().decode(chars).replace(/\0/g, '')]; | ||
| } |
Collaborator
There was a problem hiding this comment.
This is 100% redundant. You're taking text and decoding it into text? Why are you doing this? Not to mention nc_get_vara_text doesn't exist.
Comment on lines
+459
to
+476
| // Calculate total elements in the slice | ||
| const totalElements = count.reduce((a, b) => a * b, 1); | ||
| console.log(`Loading sliced array: start=[${start.join(', ')}], count=[${count.join(', ')}], total=${totalElements}, type=${arrayType}`); | ||
|
|
||
| if (!arrayType) throw new Error("Failed to get array type") | ||
| if (totalElements === 0) throw new Error("Slice size is 0") | ||
|
|
||
| // Validate start and count arrays match dimensions | ||
| if (start.length !== info.shape.length || count.length !== info.shape.length) { | ||
| throw new Error(`Dimension mismatch: variable has ${info.shape.length} dimensions, but start/count have ${start.length}/${count.length}`); | ||
| } | ||
|
|
||
| // Validate start + count doesn't exceed shape | ||
| for (let i = 0; i < start.length; i++) { | ||
| if (start[i] + count[i] > info.shape[i]) { | ||
| throw new Error(`Slice out of bounds for dimension ${i}: start=${start[i]}, count=${count[i]}, shape=${info.shape[i]}`); | ||
| } | ||
| } |
Collaborator
There was a problem hiding this comment.
All of this code is redundant. If something doesn't fit in a slice the output says it.
| * @param currentPath - Current path (used internally for recursion) | ||
| * @returns Flat dictionary with full variable paths | ||
| */ | ||
| export function getAllVariablesRecursive( |
Comment on lines
+370
to
+394
| if (arrayType === 2) { | ||
| const textData = module.nc_get_var_text(workingNcid, varid, arraySize); | ||
| if (textData.result !== NC_CONSTANTS.NC_NOERR) { | ||
| throw new Error(`nc_get_var_text failed with error code: ${textData.result}`); | ||
| } | ||
| if (!textData.data) { | ||
| throw new Error("nc_get_var_text returned no data"); | ||
| } | ||
| const chars = textData.data; | ||
| const shape = info.shape; | ||
| const decoder = new TextDecoder(); | ||
| if (shape.length <= 1) { | ||
| return [decoder.decode(chars).replace(/\0/g, '')]; | ||
| } | ||
| const strLen = shape[shape.length - 1]; | ||
| if (strLen === 0) return []; | ||
| const numStrings = chars.length / strLen; | ||
| const strings: string[] = []; | ||
| for (let i = 0; i < numStrings; i++) { | ||
| const start = i * strLen; | ||
| const end = start + strLen; | ||
| strings.push(decoder.decode(chars.subarray(start, end)).replace(/\0/g, '')); | ||
| } | ||
| return strings; | ||
| } |
Comment on lines
+397
to
+415
| if (rank === 0) { | ||
| if (arrayType === 3) arrayData = module.nc_get_var_short(workingNcid, varid, arraySize); | ||
| else if (arrayType === 4) arrayData = module.nc_get_var_int(workingNcid, varid, arraySize); | ||
| else if (arrayType === 10) arrayData = module.nc_get_var_longlong(workingNcid, varid, arraySize); | ||
| else if (arrayType === 5) arrayData = module.nc_get_var_float(workingNcid, varid, arraySize); | ||
| else if (arrayType === 6) arrayData = module.nc_get_var_double(workingNcid, varid, arraySize); | ||
| else arrayData = module.nc_get_var_double(workingNcid, varid, arraySize); | ||
| } | ||
| else { | ||
| const start = new Array(rank).fill(0); | ||
| const count = [...info.shape]; | ||
|
|
||
| if (arrayType === 3) arrayData = module.nc_get_vara_short(workingNcid, varid, start, count); | ||
| else if (arrayType === 4) arrayData = module.nc_get_vara_int(workingNcid, varid, start, count); | ||
| else if (arrayType === 10) arrayData = module.nc_get_vara_longlong(workingNcid, varid, start, count); | ||
| else if (arrayType === 5) arrayData = module.nc_get_vara_float(workingNcid, varid, start, count); | ||
| else if (arrayType === 6) arrayData = module.nc_get_vara_double(workingNcid, varid, start, count); | ||
| else arrayData = module.nc_get_vara_double(workingNcid, varid, start, count); | ||
| } |
Collaborator
There was a problem hiding this comment.
I don't understand why you are making calls to vara here? This is anti-ethical to the point of this function which is to grab all of the data. So why are you slicing it?
Member
Author
|
yes, all |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
all others worked but cases like this:
related to EarthyScience/Browzarr#549
@TheJeran I don't see how -136 is a NCID OR a groupid issue anymore. It looks more like a shape/dim mismatch. Probably you will have some insights after taking a look at the issue. I can read other variables, 0D, 3D, 4D,... but 1D and 2D.