Added typings to annoMatrix dir. (#2365)

[MillenniumFalconMechanic]

Reviewers

Functional:
@colinmegill, @seve, @tihuan

---

Changes

• Added typings to src/annoMatrix directory.
• Fixed compile errors in tests introduced by adding typings to src/annoMatrix directory (typings in tests are still to do).
• Updated actions/embedding.ts to fix compile error introduced by using Field.emb rather than "emb" .
• I have marked unresolved suppressed compile errors with "TODO: revisit" where it looks like code changes may be required to fix. I would like to validate the approach in these spots; whether to keep the suppression or what approach to take if changes are desired. The areas I have questions about are:
  • annomatrix.ts L225 & L227 - 229
  • annomatrix.ts L484
  • annomatrix.ts L620
  • annomatrix.ts L644
  • annomatrix.ts L655
  • crossfilter.ts L207
  • crossfilter.ts L296
  • crossfilter.ts L321
  • crossfilter.ts L347
  • loader.ts L211
  • loader.ts L234
  • loader.ts L189
  • loader.ts L193
  • loader.ts L252
  • loader.ts L242
  • loader.ts L235
  • loader.ts L378
  • schema.ts L38
  • views.ts L46
  • views.ts L148
  • views.ts L151
  • views.ts L159
  • wherecache.ts L169
  • wherecache.ts L172
• I have marked a few areas of the code with "TODO: waiting for typings" where typings from another folder is required.
• I made a few small code changes and have marked these with comments on this PR.

[codecov]

Codecov Report

Merging #2371 (ebf1215) into main (08b03ac) will not change coverage.
The diff coverage is n/a.

❗ Current head ebf1215 differs from pull request most recent head 9103cad. Consider uploading reports for the commit 9103cad to get more accurate results

@@           Coverage Diff           @@
##             main    #2371   +/-   ##
=======================================
  Coverage   68.23%   68.23%           
=======================================
  Files         127      127           
  Lines       12259    12259           
=======================================
  Hits         8365     8365           
  Misses       3894     3894           

Flag	Coverage Δ
frontend	66.99% <ø> (ø)
javascript	66.99% <ø> (ø)
smokeTest	∅ <ø> (∅)
smokeTestAnnotations	∅ <ø> (∅)
unitTest	66.99% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 08b03ac...9103cad. Read the comment docs.

[tihuan]

Just went through all the files! Thanks so much for adding all the types 🙏 It's incredibly helpful for understanding what's going on now 🤩

For my own education, how did you figure out when a function accepts/returns different typed arrays?

For example, client/src/annoMatrix/annoMatrix.ts:428:

abstract setObsColumnValues(
    col: string,
    obsLabels: Int32Array, // <-- how did you figure out this is Int32Array? 
    value: ObsColumnValue
  ): Promise<AnnoMatrix>;

or:

client/src/annoMatrix/viewCreators.ts:16:

export function isubsetMask(
  annoMatrix: AnnoMatrix,
  obsMask: Uint8Array // <---- how did you figure this out?
): AnnoMatrixRowSubsetView {
  /*
		Subset annomatrix to contain the rows which have truish value in the mask.
    Maks length must equal annoMatrix.nObs (row count).
	*/
  return isubset(annoMatrix, _maskToList(obsMask));
}

Thanks again!

It definitely feels like a puzzle and you nailed it 🔥 🙏 !

[MillenniumFalconMechanic]

For my own education, how did you figure out when a function accepts/returns different typed arrays?

For example, client/src/annoMatrix/annoMatrix.ts:428:

abstract setObsColumnValues(
    col: string,
    obsLabels: Int32Array, // <-- how did you figure out this is Int32Array? 
    value: ObsColumnValue
  ): Promise<AnnoMatrix>;

or:

client/src/annoMatrix/viewCreators.ts:16:

export function isubsetMask(
  annoMatrix: AnnoMatrix,
  obsMask: Uint8Array // <---- how did you figure this out?
): AnnoMatrixRowSubsetView {
  /*
		Subset annomatrix to contain the rows which have truish value in the mask.
    Maks length must equal annoMatrix.nObs (row count).
	*/
  return isubset(annoMatrix, _maskToList(obsMask));
}

Hi Timmy, when working through the typings, I did a combination of looking at the code itself, looking at the comments in the code, looking at possible values in the tests and also inspecting values at runtime. I suspect obsLabels: Int32Array and obsMask: Uint8Array came from inspecting the code at runtime. I may not have exercised all of the possible paths or values but this is my current best estimate at the types!

[tihuan]

For my own education, how did you figure out when a function accepts/returns different typed arrays?
For example, client/src/annoMatrix/annoMatrix.ts:428:

abstract setObsColumnValues(
    col: string,
    obsLabels: Int32Array, // <-- how did you figure out this is Int32Array? 
    value: ObsColumnValue
  ): Promise<AnnoMatrix>;

or:
client/src/annoMatrix/viewCreators.ts:16:

export function isubsetMask(
  annoMatrix: AnnoMatrix,
  obsMask: Uint8Array // <---- how did you figure this out?
): AnnoMatrixRowSubsetView {
  /*
		Subset annomatrix to contain the rows which have truish value in the mask.
    Maks length must equal annoMatrix.nObs (row count).
	*/
  return isubset(annoMatrix, _maskToList(obsMask));
}

Hi Timmy, when working through the typings, I did a combination of looking at the code itself, looking at the comments in the code, looking at possible values in the tests and also inspecting values at runtime. I suspect obsLabels: Int32Array and obsMask: Uint8Array came from inspecting the code at runtime. I may not have exercised all of the possible paths or values but this is my current best estimate at the types!

Wow that's so thorough and totally makes sense 🤩 Thanks for sharing your strategy!

Yeah @bkmartinjr is back now, so let's get his eyes on this too 👏 🎉

[tihuan]

@bkmartinjr Hi Bruce!! Can you help review this PR to see what types we need to adjust? Thanks so much!!

[bkmartinjr]

See comment in query.ts about fixing the definition of Query.

I recommend Query be a singleton query, and this function be redefined as:

fetch(field: Field, q: Query | Query[])

prefetch() and _fetch() need the same edit.

[bkmartinjr]

Looking great - only one issue remaining, plus two nits:

• Issue: the viewOf change introduced a bug into middleware.ts (gc). Left a coment.
• Nit: suggestion on how to remove an error suppression in viewCreators.ts::_maskToList()
• Nit: suggestion of how to resolve the error in the type signature for crossfilter::fillByIsSelected()

Other than those (especially the first), LGTM!

[tihuan]

@bkmartinjr this looks like the opposite of the original assertion (from undefined to now annomatrix), does this indicate runtime code change or something else? Thank you!

[bkmartinjr]

@tihuan - please review Mim's comments and discussion for an explanation. The runtime changed.

[MillenniumFalconMechanic]

@tihuan, discussion is here.

[tihuan]

Oh perfect! Thanks so much, both 🙏

[bkmartinjr]

LGTM

[tihuan]

Hmm yeah interesting that TS is saying toDrop Int32Array | (string | number)[] is not callable 🤔 Feels like a TS bug 😂

[MillenniumFalconMechanic]

@tihuan, I need to spend some time looking through this and this but was thinking they might be applicable to a couple of suppressions I have added.

[tihuan]

Ooh thanks so much for research, Mim! Ahh I see, so it sounds like Int32Array and a regular array have difference reduce signatures, because Int32Array can guarantee T to be number, but regular array's T can be number | string, and the array arg type is different too array: Int32Array vs. array: T[]. So TS can't type anything for us in this case. But the error message not callable definitely seems misleading 😆

// Int32Array
reduce(callbackfn: (previousValue: number, currentValue: number, currentIndex: number, array: Int32Array) => number, initialValue: number): number;

// Array
reduce(callbackfn: (previousValue: T, currentValue: T, currentIndex: number, array: T[]) => T, initialValue: T): T;

[tihuan]

ALL DONE!! Thanks so much for all the amazing typings, @MillenniumFalconMechanic 🔥 You are the coolest!!