feat: replace narray with numo-narray - #120
feat: replace narray with numo-narray#120Oleksii Leonov (oleksii-leonov) wants to merge 3 commits into
Conversation
|
Issue #55. |
| end | ||
| end | ||
|
|
||
| # DEPRECATED: Use Numo::NArray directly instead. |
|
Just a note for any future 👀 that this contains API-breaking changes, so should cause a major version bump. |
Isn't the convention usually to have the maintainers (us) bump the version? |
Yeah. But just wanted to call this out. |
|
Oleksii Leonov (@oleksii-leonov) could you rebase and we'll get this merged? |
df08ac8 to
4e35ef6
Compare
There was a problem hiding this comment.
Pull request overview
This PR completes the migration from the discontinued narray gem to numo-narray, updating internal helpers and raster APIs to construct/return Numo::NArray instances with more precise GDAL-to-array type mappings (notably for UInt16/UInt32), and removing the old narray dependency and monkeypatch.
Changes:
- Replace
narrayrequires/usages withnumo-narrayacross GDAL/OGR extension points. - Update GDAL data type → array type mapping to use
Numo::*classes with improved precision for unsigned integer types. - Update unit specs to reflect
numo-narraybehavior and APIs.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| spec/unit/gdal/internal_helpers_spec.rb | Updates expectations to Numo::* types and Numo::NArray allocation semantics. |
| spec/unit/gdal/extensions/raster_band/io_extensions_spec.rb | Adjusts raster-band IO expectations for Numo::NArray indexing/serialization. |
| spec/unit/gdal/extensions/raster_band/extensions_spec.rb | Updates RasterBand#to_na expectations to Numo::* typed arrays. |
| spec/unit/gdal/extensions/raster_band_classifier_spec.rb | Updates classifier setup away from indgen! for numo-narray. |
| lib/ogr/coordinate_transformation.rb | Removes narray require (no longer needed). |
| lib/gdal/raster_band.rb | Removes narray require (migration cleanup). |
| lib/gdal/internal_helpers.rb | Introduces numo-narray type mapping and deprecates legacy NArray mapping helpers. |
| lib/gdal/grid.rb | Switches to numo/narray and updates type checks/docs. |
| lib/gdal/extensions/raster_band/extensions.rb | Makes to_na/to_nna return Numo::NArray and updates projected-points allocation. |
| lib/gdal/extensions/gridder.rb | Removes narray require (migration cleanup). |
| lib/gdal/extensions/dataset/extensions.rb | Switches dataset-level to_na to build Numo::NArray output. |
| lib/ffi-gdal.rb | Removes ext/narray_ext load. |
| lib/ext/narray_ext.rb | Deletes legacy NArray#type monkeypatch. |
| ffi-gdal.gemspec | Drops narray dependency and adds numo-narray. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
numo-narray hasn't had a release since 2022. numo-narray-alt is a maintained drop in replacement. Should we consider switching? |
sounds good to me. |
|
Yes, |
4e35ef6 to
4c85b49
Compare
More pins for behavior the narray-to-numo-narray migration (telus-agcg#120) touches, so its changes surface as explicit spec diffs: * RasterBand#projected_points (never covered): nested representation is [line][pixel][coord] as documented, indexed [coord, x, y], and the output is typed after the band's pixel data type -- so integer bands truncate fractional projected coordinates (a defect to fix separately); * RasterBand#to_nna (never covered): Numo::NArray[*rows] infers Int32 from Ruby integers instead of honoring the band's byte type; * GDAL._narray_from_data_type: narray has no unsigned types and aliases FLOAT/COMPLEX to the double variants, so GDT_UInt16/GDT_UInt32 allocate signed int (UInt32 > 2**31 - 1 unrepresentable), GDT_Float32 double, GDT_CFloat32 double-complex. NArray#== ignores types, so the existing eq-based examples could not pin any of this; * Grid#create: points may be a typed NArray, not only a plain Array.
ad38eac to
7277917
Compare
|
Steve Loveless (@turboladen) Randy Stoller (@tindron)
|
More pins for behavior the narray-to-numo-narray migration (telus-agcg#120) touches, so its changes surface as explicit spec diffs: * RasterBand#projected_points (never covered): nested representation is [line][pixel][coord] as documented, indexed [coord, x, y], and the output is typed after the band's pixel data type -- so integer bands truncate fractional projected coordinates (a defect to fix separately); * RasterBand#to_nna (never covered): Numo::NArray[*rows] infers Int32 from Ruby integers instead of honoring the band's byte type; * GDAL._narray_from_data_type: narray has no unsigned types and aliases FLOAT/COMPLEX to the double variants, so GDT_UInt16/GDT_UInt32 allocate signed int (UInt32 > 2**31 - 1 unrepresentable), GDT_Float32 double, GDT_CFloat32 double-complex. NArray#== ignores types, so the existing eq-based examples could not pin any of this; * Grid#create: points may be a typed NArray, not only a plain Array.
3ef79c5 to
268bc29
Compare
More pins for behavior the narray-to-numo-narray migration (telus-agcg#120) touches, so its changes surface as explicit spec diffs: * RasterBand#projected_points (never covered): nested representation is [line][pixel][coord] as documented, indexed [coord, x, y], and the output is typed after the band's pixel data type -- so integer bands truncate fractional projected coordinates (a defect to fix separately); * RasterBand#to_nna (never covered): Numo::NArray[*rows] infers Int32 from Ruby integers instead of honoring the band's byte type; * GDAL._narray_from_data_type: narray has no unsigned types and aliases FLOAT/COMPLEX to the double variants, so GDT_UInt16/GDT_UInt32 allocate signed int (UInt32 > 2**31 - 1 unrepresentable), GDT_Float32 double, GDT_CFloat32 double-complex. NArray#== ignores types, so the existing eq-based examples could not pin any of this; * Grid#create: points may be a typed NArray, not only a plain Array.
268bc29 to
6fcc707
Compare
More pins for behavior the narray-to-numo-narray migration (telus-agcg#120) touches, so its changes surface as explicit spec diffs: * RasterBand#projected_points (never covered): nested representation is [line][pixel][coord] as documented, indexed [coord, x, y], and the output is typed after the band's pixel data type -- so integer bands truncate fractional projected coordinates (a defect to fix separately); * RasterBand#to_nna (never covered): Numo::NArray[*rows] infers Int32 from Ruby integers instead of honoring the band's byte type; * GDAL._narray_from_data_type: narray has no unsigned types and aliases FLOAT/COMPLEX to the double variants, so GDT_UInt16/GDT_UInt32 allocate signed int (UInt32 > 2**31 - 1 unrepresentable), GDT_Float32 double, GDT_CFloat32 double-complex. NArray#== ignores types, so the existing eq-based examples could not pin any of this; * Grid#create: points may be a typed NArray, not only a plain Array.
6fcc707 to
cf78a87
Compare
More pins for behavior the narray-to-numo-narray migration (telus-agcg#120) touches, so its changes surface as explicit spec diffs: * RasterBand#projected_points (never covered): nested representation is [line][pixel][coord] as documented, indexed [coord, x, y], and the output is typed after the band's pixel data type -- so integer bands truncate fractional projected coordinates (a defect to fix separately); * RasterBand#to_nna (never covered): Numo::NArray[*rows] infers Int32 from Ruby integers instead of honoring the band's byte type; * GDAL._narray_from_data_type: narray has no unsigned types and aliases FLOAT/COMPLEX to the double variants, so GDT_UInt16/GDT_UInt32 allocate signed int (UInt32 > 2**31 - 1 unrepresentable), GDT_Float32 double, GDT_CFloat32 double-complex. NArray#== ignores types, so the existing eq-based examples could not pin any of this; * Grid#create: points may be a typed NArray, not only a plain Array.
cf78a87 to
e934024
Compare
More pins for behavior the narray-to-numo-narray migration (telus-agcg#120) touches, so its changes surface as explicit spec diffs: * RasterBand#projected_points (never covered): nested representation is [line][pixel][coord] as documented, indexed [coord, x, y], and the output is typed after the band's pixel data type -- so integer bands truncate fractional projected coordinates (a defect to fix separately); * RasterBand#to_nna (never covered): Numo::NArray[*rows] infers Int32 from Ruby integers instead of honoring the band's byte type; * GDAL._narray_from_data_type: narray has no unsigned types and aliases FLOAT/COMPLEX to the double variants, so GDT_UInt16/GDT_UInt32 allocate signed int (UInt32 > 2**31 - 1 unrepresentable), GDT_Float32 double, GDT_CFloat32 double-complex. NArray#== ignores types, so the existing eq-based examples could not pin any of this; * Grid#create: points may be a typed NArray, not only a plain Array.
e934024 to
81a4591
Compare
More pins for behavior the narray-to-numo-narray migration (telus-agcg#120) touches, so its changes surface as explicit spec diffs: * RasterBand#projected_points (never covered): nested representation is [line][pixel][coord] as documented, indexed [coord, x, y], and the output is typed after the band's pixel data type -- so integer bands truncate fractional projected coordinates (a defect to fix separately); * RasterBand#to_nna (never covered): Numo::NArray[*rows] infers Int32 from Ruby integers instead of honoring the band's byte type; * GDAL._narray_from_data_type: narray has no unsigned types and aliases FLOAT/COMPLEX to the double variants, so GDT_UInt16/GDT_UInt32 allocate signed int (UInt32 > 2**31 - 1 unrepresentable), GDT_Float32 double, GDT_CFloat32 double-complex. NArray#== ignores types, so the existing eq-based examples could not pin any of this; * Grid#create: points may be a typed NArray, not only a plain Array.
81a4591 to
5d166d2
Compare
More pins for behavior the narray-to-numo-narray migration (telus-agcg#120) touches, so its changes surface as explicit spec diffs: * RasterBand#projected_points (never covered): nested representation is [line][pixel][coord] as documented, indexed [coord, x, y], and the output is typed after the band's pixel data type -- so integer bands truncate fractional projected coordinates (a defect to fix separately); * RasterBand#to_nna (never covered): Numo::NArray[*rows] infers Int32 from Ruby integers instead of honoring the band's byte type; * GDAL._narray_from_data_type: narray has no unsigned types and aliases FLOAT/COMPLEX to the double variants, so GDT_UInt16/GDT_UInt32 allocate signed int (UInt32 > 2**31 - 1 unrepresentable), GDT_Float32 double, GDT_CFloat32 double-complex. NArray#== ignores types, so the existing eq-based examples could not pin any of this; * Grid#create: points may be a typed NArray, not only a plain Array.
5d166d2 to
6bf2fdb
Compare
More pins for behavior the narray-to-numo-narray migration (telus-agcg#120) touches, so its changes surface as explicit spec diffs: * RasterBand#projected_points (never covered): nested representation is [line][pixel][coord] as documented, indexed [coord, x, y], and the output is typed after the band's pixel data type -- so integer bands truncate fractional projected coordinates (a defect to fix separately); * RasterBand#to_nna (never covered): Numo::NArray[*rows] infers Int32 from Ruby integers instead of honoring the band's byte type; * GDAL._narray_from_data_type: narray has no unsigned types and aliases FLOAT/COMPLEX to the double variants, so GDT_UInt16/GDT_UInt32 allocate signed int (UInt32 > 2**31 - 1 unrepresentable), GDT_Float32 double, GDT_CFloat32 double-complex. NArray#== ignores types, so the existing eq-based examples could not pin any of this; * Grid#create: points may be a typed NArray, not only a plain Array.
6bf2fdb to
9955be3
Compare
More pins for behavior the narray-to-numo-narray migration (telus-agcg#120) touches, so its changes surface as explicit spec diffs: * RasterBand#projected_points (never covered): nested representation is [line][pixel][coord] as documented, indexed [coord, x, y], and the output is typed after the band's pixel data type -- so integer bands truncate fractional projected coordinates (a defect to fix separately); * RasterBand#to_nna (never covered): Numo::NArray[*rows] infers Int32 from Ruby integers instead of honoring the band's byte type; * GDAL._narray_from_data_type: narray has no unsigned types and aliases FLOAT/COMPLEX to the double variants, so GDT_UInt16/GDT_UInt32 allocate signed int (UInt32 > 2**31 - 1 unrepresentable), GDT_Float32 double, GDT_CFloat32 double-complex. NArray#== ignores types, so the existing eq-based examples could not pin any of this; * Grid#create: points may be a typed NArray, not only a plain Array.
9955be3 to
3cedeb0
Compare
Dataset#to_na has never had spec coverage, and its doc example promised per-pixel band-value tuples ([[0, 10, 99, 2], ...]) that no implementation in the repo's history ever produced. Pin what the method actually returns before the narray-to-numo-narray migration changes it, so that migration's behavior changes show up as explicit spec diffs rather than silent semantics drift: * the result is indexed [y, x, band], but its nested (#to_a) representation is band-major with x/y swapped: [band][x][y]; * the NMatrix intermediate widens types: byte bands come back as int; * a :GDT_Float32 conversion comes back double, because narray aliases NArray::FLOAT to DFLOAT. The doc comment is corrected to describe this real behavior.
More pins for behavior the narray-to-numo-narray migration (telus-agcg#120) touches, so its changes surface as explicit spec diffs: * RasterBand#projected_points (never covered): nested representation is [line][pixel][coord] as documented, indexed [coord, x, y], and the output is typed after the band's pixel data type -- so integer bands truncate fractional projected coordinates (a defect to fix separately); * RasterBand#to_nna (never covered): Numo::NArray[*rows] infers Int32 from Ruby integers instead of honoring the band's byte type; * GDAL._narray_from_data_type: narray has no unsigned types and aliases FLOAT/COMPLEX to the double variants, so GDT_UInt16/GDT_UInt32 allocate signed int (UInt32 > 2**31 - 1 unrepresentable), GDT_Float32 double, GDT_CFloat32 double-complex. NArray#== ignores types, so the existing eq-based examples could not pin any of this; * Grid#create: points may be a typed NArray, not only a plain Array.
Replace the unmaintained narray gem with numo-narray across the raster API. Numo is row-major, so RasterBand#to_na returns [y, x] (was [x, y]) and Dataset#to_na / #projected_points nesting changes accordingly; the behavior pins added by the preceding test commits turn every observable change into an explicit spec diff: * RasterBand#to_na: shape [y, x]; conversions honor the requested type exactly -- GDT_UInt16/GDT_UInt32 get real unsigned storage (narray allocated signed int; UInt32 > 2**31 - 1 was unrepresentable), GDT_Float32 gets true single precision (narray aliased FLOAT to double), GDT_CFloat32 gets single-precision complex. * RasterBand#to_nna: now an alias of #to_na, honoring the band's data type (was Numo::NArray[*rows], which let Ruby integers infer Int32). * RasterBand#projected_points: still indexed [coord, x, y] with band-typed (truncating) storage, but the nested representation is now [coord][pixel][line]; the doc is corrected to say so. * Dataset#to_na: stacks bands with Numo::NArray.dstack into documented per-pixel band-value tuples, [y, x, band], preserving dtype. The old NMatrix construction returned band-major [band][x][y] nesting and upcast types; the first numo rewrite of it raised TypeError on every call (Array#transpose on Numo arrays) -- it now has spec coverage. * Grid#create: points may be a typed Numo::NArray (was NArray). * GDAL._narray_from_data_type with no size args returns a [0]-shaped array (was an empty, shapeless NArray).
3cedeb0 to
911f76c
Compare
Depends on #151 -- this branch is stacked on
test/pin-dataset-to-na-narray-behavior; once #151 merges, the diff here shrinks to the single port commit.Replacing
narraywithnumo-narray. The narray gem is C code unmaintained for 10 years; numo-narray is its maintained successor. This is a breaking change for consumers -- #151 pins the old behavior in specs first, so every observable change shows up in this PR as an explicit spec diff rather than silent semantics drift.Behavior changes (each visible as a spec diff)
RasterBand#to_nashape[x, y][y, x](row-major)to_na(:GDT_UInt16)NArray::INT(wasted memory)Numo::UInt16to_na(:GDT_UInt32)NArray::INT(values > 2^31 - 1 overflow)Numo::UInt32to_na(:GDT_Float32)NArray::FLOATaliases DFLOAT)Numo::SFloat)to_na(:GDT_CFloat32)Numo::SComplexRasterBand#to_nnaNumo::NArray[*rows]-- Ruby integers infer Int32#to_na, honors band data typeRasterBand#projected_points[coord, x, y], nested[line][pixel][coord][coord][pixel][line](doc updated)Dataset#to_na[band][x][y]nesting, NMatrix upcast byte to intNumo::NArray.dstack: per-pixel tuples[y, x, band], dtype preservedGrid#createpointsArrayorNArrayArrayorNumo::NArrayGDAL._narray_from_data_type()(no size)[0]-shaped arrayType-mapping is now exact:
Notes for reviewers
Dataset#to_nahad no coverage and its previous doc example promised output no implementation ever produced; it now has specs and honest docs (see test(gdal): pin Dataset#to_na NArray behavior #151 for the archaeology).projected_pointsstill truncates coordinates on integer bands (pre-existing; faithful port). A follow-up PR will switch coordinate storage toNumo::DFloat.Dataset#to_na.to_aorprojected_points.to_amust swap loop nesting; code using indexed access ([y, x, band]/[coord, x, y]) is unaffected.Why Dataset#to_na changes shape here, and what is deliberately NOT changed
This PR's goal is a gem swap that keeps behavior as-is wherever numo permits.
Dataset#to_nais the one place a behavior-identical port is impossible:NMatrix, which is removed together with narray, so the method had to be rewritten either way (see test(gdal): pin Dataset#to_na NArray behavior #151 for the archaeology: the transpose was a 2014 zip-bands-into-pixels idiom that never actually produced its documented per-pixel output on real 2-D bands).#to_arepresentation -- never both.result[y, x, band]returns the same element before and after (the pinned "is indexed as [y, x, band]" spec passes unchanged on both sides). The nested representation changing from band-major[band][x][y]to per-pixel[y][x][band]tuples is the forced consequence -- which happens to be what the 2014 doc example always promised.Any further semantic redesign -- ordering guarantees, a different stacking layout, the
projected_pointscoordinate typing/nesting -- is deliberately out of scope and deferred to separate follow-up PRs, so this one stays reviewable as "same behavior, new backing gem, honest specs".Put concretely for Dataset#to_na consumers: reading one pixel's band values --
result[y, x, true]-- worked before this PR (the old index order accidentally fulfilled the documented "points per pixel" intent) and works identically after it; the pinned spec example for it passes unchanged on both sides of the stack. What this PR changes is only the nested representation:#to_a/iteration used to show band-major[band][x][y]planes and now shows the same per-pixel tuples the indexed access always gave, so the two views of the data finally agree with each other and with the docs.