gosling.js icon indicating copy to clipboard operation
gosling.js copied to clipboard

refactor: add types for `tile` and `tileData` in `gosling-track.ts`

Open sehilyi opened this issue 2 years ago • 1 comments

Toward #800 (I think typing out tileData will enable us to deal with this issue better in the future)

Change List

  • Specify types for tile and tileData which was mostly any in the gosling-track.ts

Checklist

  • [ ] Ensure the PR works with all demos on the online editor
  • [ ] Unit tests added or updated
  • [ ] Examples added or updated
  • [ ] Documentation updated (e.g., added API functions)
  • [ ] Screenshots for visual changes (e.g., new encoding support or UI change on Editor)

sehilyi avatar Aug 11 '22 19:08 sehilyi

Thank you for the detailed explanations. My type definition was incorrect since gos was required instead of optional, and making it optional gives more type errors.

For context, these are the main data objects that we currently use.

  • tile.tabularData: Tabular data received directly from Gosling data fetchers
  • tile.tileData: Sparse HiGlass data (e.g., beddb)
  • tile.tileData.dense: An array of numbers (i.e., number[]) of dense HiGlass data (e.g., multivec).

We currently store data from these different sources to tile.gos.tabularData for unified rendering after calling getTabularData().

I think we can entirely remove the tile.gos object (which was largely redundant with tile.tileData) and instead add a new variable in GoslingTrackClass that stores the processed tabular data:

tabularizedDataByTileId: Record<string, Record<string, Datum[]>>;

We can use tile.tileId as a key.

sehilyi avatar Aug 17 '22 16:08 sehilyi

@manzt I've made further changes to (1) remove tile.gos entirely, (2) do not mutation tile at all and instead use a separate variable private processedTileInfo: Record<string, ProcessedTileInfo>; which stores processed tabular data of tiles, (3) fix the displace issue (#800).

Would you be able to review again this PR to see if further updates make sense?

The most important changes are in src/gosling-track/gosling-track.ts and src/missing-types.d.ts.

sehilyi avatar Aug 22 '22 17:08 sehilyi

Thanks for the feedback again! I fixed several issues you found (e.g., unintentionally overwriting transformed data). There are several improvements that we can make, and I think we can work on them in follow-up PRs.

Below is a high-level summary of how tiles are processed and rendered in gosling tracks. This may not explain everything, and I think we can gradually improve this as we make updates on the gosling-track.ts file.

Untitled-2022-08-23-1419

https://excalidraw.com/#json=_S-yFMT_aK9mcpPVHOV_t,BlurYdpBgXknzSGNpEN1bQ

Steps

  1. Generate tabular data for all tiles (generateTabularData())
  2. If needed, Combine tabular data of all tiles and store it to the first tile (combineAllTilesIfNeeded())
  3. Apply data transformations to the tabular data and generate track models using it (transformDataAndCreateModels(tile);)
  4. Each of the track models is rendered separately by drawTile(tile);

sehilyi avatar Aug 24 '22 15:08 sehilyi