VDJ Integration block implementation - #1
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request delivers the foundational implementation of the VDJ Integration block. Its primary purpose is to enable researchers to match VDJ clonotypes between two distinct datasets, such as bulk and single-cell sequencing data. The block facilitates the identification of shared clonotypes and generates bidirectional linker columns, which are crucial for subsequent downstream analyses like lead selection and SHM trees. This initial release establishes the core functionality, data model, user interface, and underlying computational workflow for this new analytical tool. Highlights
Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive VDJ Integration block, structured as a multi-package monorepo for the model, UI, workflow, and software components. The overall implementation is robust, with a clear separation of concerns. My review provides feedback on improving the performance and clarity of the Python matching script, addresses a potential dependency issue that could block builds, and emphasizes the importance of adding tests for this new functionality.
| @@ -0,0 +1 @@ | |||
| polars-lts-cpu==1.33.1 | |||
There was a problem hiding this comment.
The version 1.33.1 for polars-lts-cpu appears to be incorrect. This version is not available on PyPI and does not follow the package's versioning scheme, which will likely cause the software build to fail. Please correct it to a valid version. For reference, a recent version of polars-lts-cpu is 0.20.31.
| return pl.read_csv(path, separator="\t") | ||
|
|
||
|
|
||
| def melt_chains(df: pl.DataFrame, prefix: str) -> pl.DataFrame: |
There was a problem hiding this comment.
The prefix parameter is unused within this function and can be removed for improved code clarity. Remember to update the calls to this function in main.
| def melt_chains(df: pl.DataFrame, prefix: str) -> pl.DataFrame: | |
| def melt_chains(df: pl.DataFrame) -> pl.DataFrame: |
| pl.col("vGene").map_elements(strip_allele, return_dtype=pl.Utf8).alias("vGene_stripped"), | ||
| pl.col("jGene").map_elements(strip_allele, return_dtype=pl.Utf8).alias("jGene_stripped"), |
There was a problem hiding this comment.
For better performance, it's recommended to use Polars' native string expressions instead of map_elements, which can be slow. The current implementation can be rewritten to use fill_null and str.replace for a more idiomatic and performant solution.
| pl.col("vGene").map_elements(strip_allele, return_dtype=pl.Utf8).alias("vGene_stripped"), | |
| pl.col("jGene").map_elements(strip_allele, return_dtype=pl.Utf8).alias("jGene_stripped"), | |
| pl.col("vGene").fill_null("").str.replace(r"\*\d+$", "").alias("vGene_stripped"), | |
| pl.col("jGene").fill_null("").str.replace(r"\*\d+$", "").alias("jGene_stripped"), |
| target_melted = melt_chains(target_df, "target") | ||
| ref_melted = melt_chains(ref_df, "reference") |
There was a problem hiding this comment.
Following the removal of the unused prefix parameter from the melt_chains function, these calls should be updated accordingly.
| target_melted = melt_chains(target_df, "target") | |
| ref_melted = melt_chains(ref_df, "reference") | |
| target_melted = melt_chains(target_df) | |
| ref_melted = melt_chains(ref_df) |
| /* | ||
| There are no tests yet, create them via blockTest from @platforma-open/sdk-test' function. | ||
| */ |
There was a problem hiding this comment.
This file indicates that no tests have been implemented yet. For a new feature of this complexity, it is crucial to add tests to ensure its correctness and facilitate future maintenance. Please consider adding unit tests for the Python matching logic and integration tests for the Tengo workflow.
fd0dfaa to
0d3d68e
Compare
Adds the missing pl7.app/linker/excludeColumns and pl7.app/label annotations to the toReference linker column, matching the spec in pcolumn-specs.md. The excludeColumns filter uses the target axis chain value to prevent discovery of same-chain columns through the linker.
0d3d68e to
787db20
Compare
2 participants
VDJ Integration block implementation