Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 3 additions & 4 deletions model/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@
"version": "2.7.1",
"description": "Block model",
"type": "module",
"main": "dist/index.cjs",
"module": "dist/index.js",
"main": "./dist/index.js",
"types": "dist/index.d.ts",
"exports": {
".": {
"sources": "./src/index.ts",
"import": "./dist/index.js",
"default": "./dist/index.js",
"types": "./dist/index.d.ts"
},
"./dist/*": "./dist/*"
Expand All @@ -23,8 +23,7 @@
},
"dependencies": {
"@platforma-sdk/model": "catalog:",
"@milaboratories/graph-maker": "catalog:",
"@milaboratories/strings": "catalog:"
"@milaboratories/graph-maker": "catalog:"
},
"devDependencies": {
"@milaboratories/ts-builder": "catalog:",
Expand Down
199 changes: 139 additions & 60 deletions model/src/index.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,29 @@
import type { GraphMakerState } from '@milaboratories/graph-maker';
import { graphMakerPlugin } from '@milaboratories/graph-maker/plugin';
import type {
BlockRenderCtx,
DataInfo,
InferOutputsType,
PColumn,
PColumnIdAndSpec,
PColumnValues,
PFrameHandle,
PlMultiSequenceAlignmentModel,
PlRef,
RenderCtx,
SUniversalPColumnId,
TreeNodeAccessor,
} from '@platforma-sdk/model';
import {
BlockModel,
BlockModelV3,
DataModelBuilder,
createPFrameForGraphs,
} from '@platforma-sdk/model';
import strings from '@milaboratories/strings';
import { getDefaultBlockLabel } from './label';

export type BlockArgs = {
// ---------------------------------------------------------------------------

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove, every time AI generates separators in different format, better just do not use them at all

// Block data versions
// ---------------------------------------------------------------------------

type OldArgs = {
defaultBlockLabel: string;
customBlockLabel: string;
inputAnchor?: PlRef;
Expand All @@ -31,19 +35,113 @@ export type BlockArgs = {
mem: number;
};

export type UiState = {
type OldUiState = {
graphStateUMAP: GraphMakerState;
alignmentModel: PlMultiSequenceAlignmentModel;
};

/** v1 block data — includes graph state that will be transferred to plugin */
type BlockDataV1 = {
defaultBlockLabel: string;
customBlockLabel: string;
inputAnchor?: PlRef;
sequencesRef: SUniversalPColumnId[];
sequenceType: 'aminoacid' | 'nucleotide';
umap_neighbors: number;
umap_min_dist: number;
cpu: number;
mem: number;
graphStateUMAP: GraphMakerState;
alignmentModel: PlMultiSequenceAlignmentModel;
};

/** v2 block data — graph state lives in plugin */
export type BlockData = {
defaultBlockLabel: string;
customBlockLabel: string;
inputAnchor?: PlRef;
sequencesRef: SUniversalPColumnId[];
sequenceType: 'aminoacid' | 'nucleotide';
umap_neighbors: number;
umap_min_dist: number;
cpu: number;
mem: number;
alignmentModel: PlMultiSequenceAlignmentModel;
};

// ---------------------------------------------------------------------------
// Plugin instances
// ---------------------------------------------------------------------------

const umapPlugin = graphMakerPlugin('scatterplot-umap').create({
pluginId: 'umap',
transferAt: 'v1',
config: {
initialTitle: 'Clonotype Space UMAP',
initialTemplate: 'dots',
initialState: {
currentTab: 'settings',
layersSettings: {
dots: {
dotFill: '#99E099',
},
},
},
},
});

// ---------------------------------------------------------------------------
// Data model
// ---------------------------------------------------------------------------

const dataModel = new DataModelBuilder()
.from<BlockDataV1>('v1')
.upgradeLegacy<OldArgs, OldUiState>(({ args, uiState }) => ({
defaultBlockLabel: args.defaultBlockLabel,
customBlockLabel: args.customBlockLabel,
inputAnchor: args.inputAnchor,
sequencesRef: args.sequencesRef,
sequenceType: args.sequenceType,
umap_neighbors: args.umap_neighbors,
umap_min_dist: args.umap_min_dist,
cpu: args.cpu,
mem: args.mem,
graphStateUMAP: uiState.graphStateUMAP,
alignmentModel: uiState.alignmentModel,
}))
.transfer(umapPlugin, (v1) => ({
state: v1.graphStateUMAP,
chartType: 'scatterplot-umap' as const,
}))
.migrate<BlockData>('v2', ({ graphStateUMAP: _, ...rest }) => rest)
.init(() => ({
defaultBlockLabel: getDefaultBlockLabel({
sequenceLabels: [],
umap_neighbors: 15,
umap_min_dist: 0.5,
}),
customBlockLabel: '',
sequenceType: 'aminoacid',
sequencesRef: [],
umap_neighbors: 15,
umap_min_dist: 0.5,
mem: 64,
cpu: 8,
alignmentModel: {},
}));
Comment on lines +117 to +131

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The init function for the data model contains several hardcoded default values (e.g., for umap_neighbors, umap_min_dist, mem, cpu). To improve maintainability and readability, it's recommended to extract these magic numbers into named constants. This makes it easier to understand their purpose and to update them in one place if needed.

For example:

const DEFAULT_UMAP_NEIGHBORS = 15;
const DEFAULT_UMAP_MIN_DIST = 0.5;
const DEFAULT_MEM = 64;
const DEFAULT_CPU = 8;

// ... in dataModel builder
.init(() => ({
  defaultBlockLabel: getDefaultBlockLabel({
    sequenceLabels: [],
    umap_neighbors: DEFAULT_UMAP_NEIGHBORS,
    umap_min_dist: DEFAULT_UMAP_MIN_DIST,
  }),
  // ...
  umap_neighbors: DEFAULT_UMAP_NEIGHBORS,
  umap_min_dist: DEFAULT_UMAP_MIN_DIST,
  mem: DEFAULT_MEM,
  cpu: DEFAULT_CPU,
  // ...
}));


// ---------------------------------------------------------------------------
// Helper
// ---------------------------------------------------------------------------

type Column = PColumn<DataInfo<TreeNodeAccessor> | TreeNodeAccessor | PColumnValues>;

type Columns = {
props: Column[];
};

function getColumns(ctx: Pick<RenderCtx<BlockArgs, UiState>, 'args' | 'resultPool'>): Columns | undefined {
const anchor = ctx.args?.inputAnchor;
function getColumns(ctx: Pick<BlockRenderCtx<unknown, BlockData>, 'data' | 'resultPool'>): Columns | undefined {
const anchor = ctx.data.inputAnchor;
if (anchor === undefined)
return undefined;

Expand All @@ -66,46 +164,32 @@ function getColumns(ctx: Pick<RenderCtx<BlockArgs, UiState>, 'args' | 'resultPoo
};
}

export const model = BlockModel.create()

.withArgs<BlockArgs>({
defaultBlockLabel: getDefaultBlockLabel({
sequenceLabels: [],
umap_neighbors: 15,
umap_min_dist: 0.5,
}),
customBlockLabel: '',
sequenceType: 'aminoacid',
sequencesRef: [],
umap_neighbors: 15,
umap_min_dist: 0.5,
mem: 64,
cpu: 8,
// ---------------------------------------------------------------------------
// Block model
// ---------------------------------------------------------------------------

export const platforma = BlockModelV3.create(dataModel)

.args((data) => {
if (!data.inputAnchor) throw new Error('Dataset is required');
if (!data.sequencesRef.length) throw new Error('Sequence columns are required');
if (data.umap_neighbors === undefined) throw new Error('Neighbors is required');
if (data.umap_min_dist === undefined) throw new Error('Minimum distance is required');
if (data.mem === undefined) throw new Error('Memory is required');
if (data.cpu === undefined) throw new Error('CPU is required');
return {
inputAnchor: data.inputAnchor,
sequencesRef: data.sequencesRef,
sequenceType: data.sequenceType,
umap_neighbors: data.umap_neighbors,
umap_min_dist: data.umap_min_dist,
cpu: data.cpu,
mem: data.mem,
defaultBlockLabel: data.defaultBlockLabel,
customBlockLabel: data.customBlockLabel,
};
})
Comment on lines +173 to 191

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The .args() function can be simplified. The validation checks are fine, but the returned object is unnecessarily verbose. Since you are returning almost all properties from data except for alignmentModel, you can use object destructuring with rest properties to make it more concise.

  .args((data) => {
    if (!data.inputAnchor) throw new Error('Dataset is required');
    if (!data.sequencesRef.length) throw new Error('Sequence columns are required');
    if (data.umap_neighbors === undefined) throw new Error('Neighbors is required');
    if (data.umap_min_dist === undefined) throw new Error('Minimum distance is required');
    if (data.mem === undefined) throw new Error('Memory is required');
    if (data.cpu === undefined) throw new Error('CPU is required');
    const { alignmentModel: _, ...args } = data;
    return args;
  })


.withUiState<UiState>({
graphStateUMAP: {
title: 'Clonotype Space UMAP',
template: 'dots',
currentTab: 'settings',
layersSettings: {
dots: {
dotFill: '#99E099',
},
},
},
alignmentModel: {},
})

.argsValid((ctx) =>
ctx.args.inputAnchor !== undefined
&& ctx.args.sequencesRef.length > 0
&& ctx.args.umap_neighbors !== undefined
&& ctx.args.umap_min_dist !== undefined
&& ctx.args.mem !== undefined
&& ctx.args.cpu !== undefined,
)

.output('inputOptions', (ctx) =>
ctx.resultPool.getOptions([{
axes: [
Expand All @@ -123,7 +207,7 @@ export const model = BlockModel.create()
)

.output('sequenceOptions', (ctx) => {
const ref = ctx.args.inputAnchor;
const ref = ctx.data.inputAnchor;
if (ref === undefined) return undefined;

const isSingleCell = ctx.resultPool.getPColumnSpecByRef(ref)?.axesSpec[1].name === 'pl7.app/vdj/scClonotypeKey';
Expand Down Expand Up @@ -196,15 +280,6 @@ export const model = BlockModel.create()
return createPFrameForGraphs(ctx, columns.props);
})

.outputWithStatus('umapPf', (ctx): PFrameHandle | undefined => {
const pCols = ctx.outputs?.resolve('umapPf')?.getPColumns();
if (pCols === undefined) {
return undefined;
}

return createPFrameForGraphs(ctx, pCols);
})

.output('umapOutput', (ctx) => ctx.outputs?.resolve('umapOutput')?.getLogHandle())

// Create a PTable with the first dimension of the UMAP to test if file is empty
Expand Down Expand Up @@ -242,14 +317,18 @@ export const model = BlockModel.create()

.title(() => 'Clonotype Space')

.subtitle((ctx) => ctx.args.customBlockLabel || ctx.args.defaultBlockLabel)
.subtitle((ctx) => ctx.data.customBlockLabel || ctx.data.defaultBlockLabel)

.sections((_ctx) => ([
{ type: 'link', href: '/', label: strings.titles.main },
{ type: 'link', href: '/', label: 'Main' },

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The label for the section link is hardcoded as 'Main'. Previously, this was sourced from @milaboratories/strings (strings.titles.main), which is better for maintainability and internationalization (i18n). While the strings dependency has been removed, hardcoding UI strings directly in the model is not ideal. Consider using a centralized place for UI strings if one exists, or re-introducing a simple mechanism for it.

]))

.done(2);
.plugin(umapPlugin, {
blockColumns: (ctx) => ctx.outputs?.resolve('umapPf')?.getPColumns(),
})

.done();

export type BlockOutputs = InferOutputsType<typeof model>;
export type BlockOutputs = InferOutputsType<typeof platforma>;

export { getDefaultBlockLabel } from './label';
7 changes: 7 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,12 @@
"typescript": "catalog:",
"shx": "catalog:"
},
"pnpm": {
"overrides": {
"@milaboratories/graph-maker": "/Users/elenaerokhina/projects/mictx/core/visualizations/packages/graph-maker/package.tgz",
"@platforma-sdk/model": "/Users/elenaerokhina/projects/mictx/core/platforma/sdk/model/package.tgz",
"@platforma-sdk/ui-vue": "/Users/elenaerokhina/projects/mictx/core/platforma/sdk/ui-vue/package.tgz"
}
},
Comment on lines +23 to +29

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The pnpm.overrides section contains hardcoded local paths specific to a user's machine. This will break the build for other developers and in CI/CD environments. This section must be removed before merging. If you need to use local packages for development, consider using pnpm link or a similar mechanism that doesn't involve checking in user-specific paths into version control.

"packageManager": "pnpm@9.12.0"
}
Loading