feat(model): add filterToAnchorPartitions option to getAnchoredPColumns#1514
feat(model): add filterToAnchorPartitions option to getAnchoredPColumns#1514PaulNewling wants to merge 5 commits intomainfrom
Conversation
When set, derives unique axis-0 partition keys from the resolved anchor column(s) and injects a pl7.app/axisKeys/0 annotation onto each returned column spec. findLabelsForColumnAxis respects this annotation, so graph-maker sample dropdowns only show keys present in the anchor dataset rather than all keys in the originating samples block.
Signed-off-by: Paul Newling <paulnewling@gmail.com>
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 introduces a Highlights
Changelog
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
|
🦋 Changeset detectedLatest commit: d21b2f7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![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 filterToAnchorPartitions option to getAnchoredPColumns, which is a useful feature for scoping data based on anchor partitions. The implementation is mostly solid, but I've found a potential logic issue when no partition keys are found, which could lead to incorrect behavior in downstream consumers. I've also suggested a small improvement for maintainability. Overall, these are valuable changes.
…tion Signed-off-by: Paul Newling <paulnewling@gmail.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds a filterToAnchorPartitions option to getAnchoredPColumns to scope partition keys based on anchor columns. The implementation is logical and correctly handles data readiness by propagating undefined. I have two suggestions for minor improvements: one to remove a redundant line of code for simplification, and another to add an early return for a micro-optimization. Please see the detailed comments.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1514 +/- ##
==========================================
+ Coverage 54.37% 54.50% +0.13%
==========================================
Files 242 242
Lines 13686 13701 +15
Branches 2820 2827 +7
==========================================
+ Hits 7442 7468 +26
+ Misses 5310 5291 -19
- Partials 934 942 +8 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Paul Newling <paulnewling@gmail.com>
Signed-off-by: Paul Newling <paulnewling@gmail.com>
1 participant
Summary
Adds a
filterToAnchorPartitionsoption toResultPool.getAnchoredPColumns.When set to
true, the method derives the unique axis-0 partition keys from the resolved anchor column(s) and injects apl7.app/axisKeys/0annotation onto each returned column spec.findLabelsForColumnAxisrespects this annotation, so graph-maker sample dropdowns are scoped to samples actually present in the selected dataset rather than all samples in the originating samples & data block.Motivation
Blocks that accept a dataset reference (e.g. a MiXCR clonotyping output) and pull upstream sampleId-keyed metadata via
getAnchoredPColumnsface a subtle scoping problem: the upstream samples & data block publishes label and metadata columns covering all of its samples, not just the subset that ended up in any one downstream dataset. Without this option, graph-maker shows the full sample list in its dropdowns, including samples that have no data in the selected dataset.Currently we are attempting to work around this issue with a work around:
ptjoin step in the workflow to produce a dataset-filtered label column..filter((c) => c.spec.name !== 'pl7.app/label')on thegetAnchoredPColumnsresult in the model.This has potential possibilities to strip labels and pollute downstream data if set incorrectly though.
(See this PR as an example of a workaround step - LINK)
filterToAnchorPartitionsreplaces pattern 2 with a single declarative option, and enables theexcludeoption to cleanly own the label-exclusion intent. The sampleId values themselves are never modified, so downstream blocks that join on the original IDs continue to work correctly.Changes
sdk/model/src/render/api.ts:filterToAnchorPartitionsfield onUniversalPColumnOptsderiveAnchorPartitionKeyshelpergetAnchoredPColumnsimplementation.Images