DefinitelyTyped / DefinitelyTyped

The repository for high quality TypeScript type definitions.
Other
48.45k stars 30.14k forks source link

NumJs typing bugs #18508

Closed huan closed 3 years ago

huan commented 7 years ago

Recently we released the numjs typing definition which is awesome. (#18343)

After I used it in my ML project, I found serial bugs, list as the following(will be updated):

  1. nj.images.read(input: string): NdArray<Uint32Array>; should be Uint8Array, according to https://github.com/nicolaspanel/numjs/blob/master/src/images/read.js#L17
  2. nj.tolist(): T[]; seems will cause problems, because most of the time, the return type will not be T[].
  3. nj.array<T = number>(arr: NjArray<T>, dtype?: BaseNdArray.DataType): NdArray<T>; seems will cause problems too, because we can construct the nj.array by number[], number[][], even number[][][], the nj.array should always have T=number.

To be updated...

taoqf commented 7 years ago

nj.tolist(): T[]; seems will cause problems, because most of the time, the return type will not be T[].

Then what will be the return type? actually i do not use this module much, it will be so kind if you could tell me the return types so i can fix this.

nj.array<T = number>(arr: NjArray, dtype?: BaseNdArray.DataType): NdArray; seems will cause problems too, because we can construct the nj.array by number[], number[][], even number[][][], the nj.array should always have T=number.

You may like use nj.array like this:

const my_array = nj.array<number[]>([[2], [3, 4]]);

Good luck.

huan commented 7 years ago

Thanks for the quick fix!

My problem is that the tolist(): T[] and nj.arrahy<T> looks tricky for me, because I'm using the array with strange shape like:

const a = nj.array([0, [1, 2], [2, 3], [[4, [5, 6]]], 4])
console.log(a.shape) // Output: [ 5 ]

const l = a.tolist()
console.log(l[1][1]) // TSError: Element implicitly has an 'any' type because type 'number | number[] | (number | number[])[][]' has no index signature. (7017)

I think maybe I should not use the nj.array with un-fixed shape because in numpy it seems always with fixed rows/cols?

taoqf commented 7 years ago

will a.tolist<number[]>(); or a.tolist() as number[][] work for you?

huan commented 7 years ago

Yes, absolutely.

Actually, this is the solution I'm using now.

What I'm confusing is that whether there has a Typing Setting will do it automatically because the Type is not static in numjs.

huan commented 7 years ago

Another bug:

function resize(img: NdArray<Uint32Array>, height: number, width: number): NdArray<Uint32Array>;

should be NdArray<Uint8Array>

Just noticed that it's already fixed. Thanks!

huan commented 7 years ago

Issue List(v2, updating)

1. NdArray property: selection

Currently, we are leaked of selection property for NdArray, so I have to use any like this: const data = (ndarray as any).selection.data as Uint8Array.

According to the suggestion of author of numjs: https://github.com/nicolaspanel/numjs/issues/21#issuecomment-319301957 and the code here: https://github.com/nicolaspanel/numjs/blob/master/src/ndarray.js#L24, we should add the selection so that we can access the buffer directly.

Data structer illustrated as the follow code & log:

// ndarray: nj.NdArray<uint8>
console.log(ndarray.selection)
View1duint8 {
  data: Uint8Array [ 1, 2, 3 ],
  shape: [ 3 ],
  stride: [ 1 ],
  offset: 0 }

2. NdArray constructor: new nj.NdArray(...)

We should export NdArray, so I can use nj.NdArray instead of (nj as any).NdArray, like the following code usage:

const b = Buffer.from(base64Text, 'base64')
const t = new Uint8ClampedArray(b)
const a = new (nj as any).NdArray(t, [512, 512, 3])
console.log((nj as any).NdArray)
// { [Function: NdArray] new: [Function: createArray] }

3. ndarray is not assignable to type NdArray

Type 'ndarray' is not assignable to type 'NdArray'. Property 'ndim' is missing in type 'ndarray'.

let image = new (nj as any).NdArray(data.data, data.width, data.height) as nj.NdArray<Uint8ClampedArray>
image = image.hi(r2, c2).lo(r1, c1) // error here

4. ndarray.hi should accept null args

Argument of type 'null' is not assignable to parameter of type 'number'. (method) ndarray<number>.hi(...args: number[]): ndarray<number>

According to https://github.com/scijs/scijs-ndarray-for-matlab-users: a(1:5, :) | a.hi(5, null) | the first five rows of a

      const image = ndarray(data.data, [data.width, data.height, 4])
      const cropedImage = (image as any).hi(r1, c1, null).lo(r0, c0, null)

5. 'NdArray' does not exist

Property 'NdArray' does not exist on type 'typeof "node_modules/@types/numjs/index"'.

console.log(nj.array([1, 2, 3, 4]).reshape(2, 2) instanceof nj.NdArray)
orta commented 3 years ago

Hi thread, we're moving DefinitelyTyped to use GitHub Discussions for conversations the @types modules in DefinitelyTyped.

To help with the transition, we're closing all issues which haven't had activity in the last 6 months, which includes this issue. If you think closing this issue is a mistake, please pop into the TypeScript Community Discord and mention the issue in the definitely-typed channel.