viv icon indicating copy to clipboard operation
viv copied to clipboard

Flatten TileSelection & RasterSelection interfaces.

Open manzt opened this issue 4 years ago • 1 comments

Originally posted by @manzt in https://github.com/hms-dbmi/viv/pull/358#discussion_r563341977

Now that we are using for the loaders, we can think about flattening the interface for getTile and getRaster into named args.

Current:

export type PixelSourceSelection<S extends string[]> = {
  [K in S[number]]: number;
};

export interface RasterSelection<S extends string[]> {
  selection: PixelSourceSelection<S>;
}

export interface TileSelection<S extends string[]> {
  x: number;
  y: number;
  selection: PixelSourceSelection<S>;
  signal?: AbortSignal;
}

export interface PixelSource<S extends string[]> {
  getRaster(sel: RasterSelection<S>): Promise<LayerData>;
  getTile(sel: TileSelection<S>): Promise<LayerData>;
  onTileError(err: Error): void;
  shape: number[];
  dtype: SupportedDtype;
  labels: Labels<S>;
  tileSize: number;
  meta?: PixelSourceMeta;
}

Proposed change:

export type PixelSourceSelection<S extends string[]> = {
  [K in S[number]]: number;
};

export interface PixelSource<S extends string[]> {
  getRaster(sel: PixelSourceSelection<S>): Promise<LayerData>;
  getTile(x: number, y: number, sel: PixelSourceSelection<S>, signal?: AbortSignal): Promise<LayerData>;
  onTileError(err: Error): void;
  shape: number[];
  dtype: SupportedDtype;
  labels: Labels<S>;
  tileSize: number;
  meta?: PixelSourceMeta;
}

manzt avatar Jan 24 '21 20:01 manzt

Following up here, I think this issue is exacerbated right now by the getVolume function coming. Right now my method looks like

getVolume(
    sel: RasterSelection<S>,
    updateProgress: () => void,
    downsampleDepth: number
  ): Promise<LayerData>;

Not withstanding what the API around downsampling will look like (I really am not sure with that one), function parameters unique to each call seem common, and may continue to grow with use-cases. For example, I could see us adding a signal parameter to getRaster to speed up rapid selection changes so that layers can abort ongoing requests. Maybe something like

getXXXXXX(sel: PixelSourceSelection<S>, opts: GetOptions<S>)

The nice thing is we could reuse signal in GetOptions for example or downsampleDepth if we ever have a situation where our getXXXXXX methods get fancier and we may need to downsample in a certain direction like we do for z in getVolume.

ilan-gold avatar Jan 25 '21 22:01 ilan-gold