Skip to content

Julenmendieta/MILAB-5143_supportMultipleChainsInRedefine - #41

Open
julenmendieta wants to merge 4 commits into
mainfrom
julenmendieta/MILAB-5143_supportMultipleChainsInRedefine
Open

Julenmendieta/MILAB-5143_supportMultipleChainsInRedefine#41
julenmendieta wants to merge 4 commits into
mainfrom
julenmendieta/MILAB-5143_supportMultipleChainsInRedefine

Conversation

@julenmendieta

Copy link
Copy Markdown
Contributor

No description provided.

Expose a spec-only PColumn that carries clonotypingRunId in its domain and propagates the dataset trace. This enables downstream blocks to discover import-vdj runs the same way they discover MiXCR runs via mixcr.com/clns, using a single
  per-block entry point to locate all associated chain tables.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates several @platforma-sdk and @milaboratories dependencies and introduces a new 'run' marker column to the result pframe in both bulk and single-cell VDJ import workflows to facilitate downstream block discovery. A critical issue was identified in both workflow files where the result.add method is missing its third argument (the data resource), which will likely lead to runtime errors during execution.

Comment on lines +371 to +382
result.add("run", runTrace.inject({
kind: "PColumn",
name: "pl7.app/vdj/run",
axesSpec: [],
domain: {
"pl7.app/vdj/clonotypingRunId": blockId
},
valueType: "String",
annotations: {
"pl7.app/label": "VDJ Import Run"
}
}))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The result.add method in the pFrameBuilder typically requires three arguments: the column key, the column specification, and the data resource. Currently, the third argument (data) is missing, which will likely cause a runtime error during workflow execution. Since this is a marker column with valueType: "String", you should provide a string value, such as blockId.

    result.add("run", runTrace.inject({
        kind: "PColumn",
        name: "pl7.app/vdj/run",
        axesSpec: [],
        domain: {
            "pl7.app/vdj/clonotypingRunId": blockId
        },
        valueType: "String",
        annotations: {
            "pl7.app/label": "VDJ Import Run"
        }
    }), blockId)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I don't believe this is correct, https://github.com/milaboratory/platforma/blob/main/sdk/workflow-tengo/src/pframes/builder.lib.tengo#L17-L19 handles when nothing is passed in for data

Comment on lines +462 to +473
result.add("run", runTrace.inject({
kind: "PColumn",
name: "pl7.app/vdj/run",
axesSpec: [],
domain: {
"pl7.app/vdj/clonotypingRunId": blockId
},
valueType: "String",
annotations: {
"pl7.app/label": "VDJ Import Run"
}
}))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

The result.add method is missing its third argument (the data resource). For a marker column of type String, you should pass a value (e.g., blockId) to ensure the column is correctly added to the PFrame and to avoid runtime errors during workflow execution.

	result.add("run", runTrace.inject({
		kind: "PColumn",
		name: "pl7.app/vdj/run",
		axesSpec: [],
		domain: {
			"pl7.app/vdj/clonotypingRunId": blockId
		},
		valueType: "String",
		annotations: {
			"pl7.app/label": "VDJ Import Run"
		}
	}), blockId)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants