Skip to content

Commit

Permalink
fix: 936 embedding bug 1
Browse files Browse the repository at this point in the history
  • Loading branch information
tihuan committed May 23, 2024
1 parent f3d49fd commit b6a7b39
Show file tree
Hide file tree
Showing 5 changed files with 78 additions and 51 deletions.
35 changes: 25 additions & 10 deletions client/src/actions/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,8 @@ import { EVENTS } from "../analytics/events";
import AnnoMatrix from "../annoMatrix/annoMatrix";
import { checkFeatureFlags } from "../util/featureFlags/featureFlags";
import { DATASET_METADATA_RESPONSE } from "../../__tests__/__mocks__/apiMock";
import { selectAvailableLayouts } from "../selectors/layoutChoice";
import { getBestDefaultLayout } from "../reducers/layoutChoice";

function setGlobalConfig(config: Config) {
/**
Expand Down Expand Up @@ -226,24 +228,37 @@ const doInitialDataLoad = (): ((
prefetchEmbeddings(annoMatrix);

const isCellGuideCxg = checkIfCellGuideCxg();

/**
* (thuang + seve) There is room to clean up by moving this to the annoMatrix initialization
*/
dispatch({
type: "annoMatrix: init complete",
annoMatrix,
obsCrossfilter,
isCellGuideCxg,
});

// save isCellGuideCxg to the reducer store
dispatch({ type: "initial data load complete", isCellGuideCxg });
const availableLayouts = selectAvailableLayouts({ annoMatrix });
const fallbackLayout = getBestDefaultLayout(availableLayouts);
const defaultLayout = config?.parameters?.default_embedding || "";

const defaultEmbedding = config?.parameters?.default_embedding;
const layoutSchema = schema?.schema?.layout?.obs ?? [];
if (
defaultEmbedding &&
layoutSchema.some((s: EmbeddingSchema) => s.name === defaultEmbedding)
) {
await dispatch(embActions.layoutChoiceAction(defaultEmbedding));
}
const finalLayout = availableLayouts.includes(defaultLayout)
? defaultLayout
: fallbackLayout;

dispatch({
type: "initial data load complete",
isCellGuideCxg,
layoutChoice: finalLayout,
});

/**
* (thuang): This dispatch is necessary to ensure that we get the right cell
* count when the page is first loaded.
* BUG: https://github.com/chanzuckerberg/single-cell-explorer/issues/936
*/
await dispatch(embActions.layoutChoiceAction(finalLayout));
} catch (error) {
dispatch({ type: "initial data load error", error });
}
Expand Down
15 changes: 3 additions & 12 deletions client/src/components/embedding/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ import actions from "../../actions";
import { getDiscreteCellEmbeddingRowIndex } from "../../util/stateManager/viewStackHelpers";
import { track } from "../../analytics";
import { EVENTS } from "../../analytics/events";
import { RootState } from "../../reducers";

// eslint-disable-next-line @typescript-eslint/no-explicit-any --- FIXME: disabled temporarily on migrate to TS.
type EmbeddingState = any;
type Props = RootState;

// @ts-expect-error ts-migrate(1238) FIXME: Unable to resolve signature of class decorator whe... Remove this comment to see the full error message
@connect((state: RootState) => ({
Expand All @@ -27,20 +27,12 @@ type EmbeddingState = any;
crossfilter: state.obsCrossfilter,
imageUnderlay: state.imageUnderlay,
}))
// eslint-disable-next-line @typescript-eslint/ban-types --- FIXME: disabled temporarily on migrate to TS.
class Embedding extends React.PureComponent<{}, EmbeddingState> {
// eslint-disable-next-line @typescript-eslint/ban-types --- FIXME: disabled temporarily on migrate to TS.
constructor(props: {}) {
super(props);
this.state = {};
}

class Embedding extends React.PureComponent<Props> {
handleLayoutChoiceClick = (): void => {
track(EVENTS.EXPLORER_LAYOUT_CHOICE_BUTTON_CLICKED);
};

handleLayoutChoiceChange = (e: React.ChangeEvent<HTMLInputElement>) => {
// @ts-expect-error ts-migrate(2339) FIXME: Property 'dispatch' does not exist on type 'Readon... Remove this comment to see the full error message
const { dispatch, imageUnderlay } = this.props;

track(EVENTS.EXPLORER_LAYOUT_CHOICE_CHANGE_ITEM_CLICKED);
Expand All @@ -59,7 +51,6 @@ class Embedding extends React.PureComponent<{}, EmbeddingState> {

// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types --- FIXME: disabled temporarily on migrate to TS.
render() {
// @ts-expect-error ts-migrate(2339) FIXME: Property 'layoutChoice' does not exist on type 'Re... Remove this comment to see the full error message
const { layoutChoice, schema, crossfilter } = this.props;
const { annoMatrix } = crossfilter || {};

Expand Down
53 changes: 29 additions & 24 deletions client/src/reducers/layoutChoice.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,23 +10,8 @@ about commonly used names. Preferentially, pick in the following order:

import type { Action, AnyAction } from "redux";
import type { RootState } from ".";
import { EmbeddingSchema, Schema } from "../common/types/schema";

function bestDefaultLayout(layouts: Array<string>): string {
const preferredNames = ["umap", "tsne", "pca"];
const idx = preferredNames.findIndex((name) => layouts.indexOf(name) !== -1);
if (idx !== -1) return preferredNames[idx];
return layouts[0];
}

function setToDefaultLayout(schema: Schema): LayoutChoiceState {
const available = schema.layout.obs
.map((v: EmbeddingSchema) => v.name)
.sort();
const current = bestDefaultLayout(available);
const currentDimNames = schema.layout.obsByName[current].dims;
return { available, current, currentDimNames };
}
import { selectAvailableLayouts } from "../selectors/layoutChoice";
import { selectSchema } from "../selectors/annoMatrix";

export interface LayoutChoiceState {
available: Array<string>;
Expand All @@ -45,19 +30,19 @@ const LayoutChoice = (
): LayoutChoiceState => {
switch (action.type) {
case "initial data load complete": {
// set default to default
const { annoMatrix } = nextSharedState;
const { layoutChoice } = action;

return {
...state,
...setToDefaultLayout(annoMatrix.schema),
available: selectAvailableLayouts(nextSharedState),
...getCurrentLayout(nextSharedState, layoutChoice),
};
}

case "set layout choice": {
const { schema } = nextSharedState.annoMatrix;
const current = (action as LayoutChoiceAction).layoutChoice;
const currentDimNames = schema.layout.obsByName[current].dims;
return { ...state, current, currentDimNames };
const { layoutChoice } = action as LayoutChoiceAction;

return { ...state, ...getCurrentLayout(nextSharedState, layoutChoice) };
}

default: {
Expand All @@ -67,3 +52,23 @@ const LayoutChoice = (
};

export default LayoutChoice;

function getCurrentLayout(
state: RootState,
layoutChoice: string
): {
current: string;
currentDimNames: Array<string>;
} {
const schema = selectSchema(state);
const currentDimNames = schema.layout.obsByName[layoutChoice].dims;

return { current: layoutChoice, currentDimNames };
}

export function getBestDefaultLayout(layouts: Array<string>): string {
const preferredNames = ["umap", "tsne", "pca"];
const idx = preferredNames.findIndex((name) => layouts.indexOf(name) !== -1);
if (idx !== -1) return preferredNames[idx];
return layouts[0];
}
15 changes: 10 additions & 5 deletions client/src/selectors/annoMatrix.ts
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
/* App dependencies */
import AnnoMatrix from "../annoMatrix/annoMatrix";
import { AnnotationColumnSchema } from "../common/types/schema";
import { RootState } from "../reducers";
import { type RootState } from "../reducers";

// eslint-disable-next-line @typescript-eslint/no-explicit-any --- update typings once annoMatrix reducer state is typed.
export const selectAnnoMatrix = (state: RootState): any => state.annoMatrix;
export const selectAnnoMatrix = (state: RootState): AnnoMatrix =>
state.annoMatrix;

/*
Returns true if user defined category has been created indicating work is in progress.
@param annoMatrix from state
@returns boolean
*/
// eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types, @typescript-eslint/no-explicit-any --- update typings once annoMatrix reducer state is typed.
export const selectIsUserStateDirty = (state: any): boolean => {
export const selectIsUserStateDirty = (state: RootState): boolean => {
const annoMatrix = selectAnnoMatrix(state);

return Boolean(
Expand All @@ -20,3 +20,8 @@ export const selectIsUserStateDirty = (state: any): boolean => {
)
);
};

export function selectSchema(state: RootState) {
const annoMatrix = selectAnnoMatrix(state);
return annoMatrix?.schema;
}
11 changes: 11 additions & 0 deletions client/src/selectors/layoutChoice.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { EmbeddingSchema } from "../common/types/schema";
import { type RootState } from "../reducers";
import { selectAnnoMatrix } from "./annoMatrix";

export function selectAvailableLayouts(state: RootState): string[] {
const annoMatrix = selectAnnoMatrix(state);

return (annoMatrix.schema?.layout?.obs || [])
.map((v: EmbeddingSchema) => v.name)
.sort();
}

0 comments on commit b6a7b39

Please sign in to comment.