feat: integrate openms-insight into quantms - #28
Conversation
Implement 5 new Streamlit analysis pipeline pages powered by the openms-insight engine to handle downstream proteomics workflows: - Data Filtering (filtering.py): * Support Low Abundance, Low Repeatability, and Low Variance filters using Polars LazyFrame. - Missing Value Imputation (imputation.py): * Connect upstream filtered datasets using session state fallback. * Provide group-aware MAR (mean/median) and technical dropout MNAR (row/global minimum) methods. - Data Normalization & Scaling (normalization.py): * Build a 3-step preprocessing chain: Mathematical transformation, column-wise sample normalization (PQn, Quantile, Reference Feature), and row-wise scaling. - Statistical Inference (statistical.py): * Dynamically route test methods (limma-like, Welch, ANOVA) based on detected group counts. * Integrate multiple testing corrections (BH, Bonferroni). - GO Enrichment Analysis (enrichment.py): * Fetch foreground lists dynamically via P-value and Log2FC constraints. * Query MyGene.info API and render interactive Plotly charts across BP, CC, and MF tabs.
📝 WalkthroughWalkthroughThe change adds filtering, imputation, normalization, statistical inference, and GO enrichment pages; changes abundance loading to return pivoted data and group mappings; migrates visualizations to OpenMS Insight components; updates Results navigation; and adds Polars runtime compatibility settings. ChangesAbundance analysis workflow
Sequence Diagram(s)sequenceDiagram
participant User
participant StreamlitPage
participant SessionState
participant AnalysisEngine
participant VisualizationComponent
User->>StreamlitPage: configure and run analysis
StreamlitPage->>AnalysisEngine: submit Polars lazy data
AnalysisEngine-->>StreamlitPage: return processed results
StreamlitPage->>SessionState: store dataframe artifact
User->>VisualizationComponent: open result page
VisualizationComponent->>SessionState: read analysis artifact
VisualizationComponent-->>User: render visualization
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (5)
content/imputation.py (1)
65-134: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an
@st.fragmentfor the imputation controls/results section.This page adds interactive widgets plus button-driven rendering without fragment scoping, so each change reruns the full page. As per coding guidelines,
**/*.py:Use@st.fragmentdecorator for interactive UI updates without full page reloads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@content/imputation.py` around lines 65 - 134, The imputation controls/results block in content/imputation.py should be scoped into a Streamlit fragment so widget changes and the Apply Imputation button do not rerun the whole page. Wrap the interactive section that builds metadata_pl, renders the selectbox/radio controls, and handles the imputation execution around impute_mar and impute_smallest_value with an `@st.fragment-decorated` function, then call that fragment from the page so the UI updates stay localized.Source: Coding guidelines
content/normalization.py (1)
79-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPut the normalization UI into an
@st.fragment.This workflow is entirely widget-driven, but it is still executed at page scope, so every interaction reruns the whole page. As per coding guidelines,
**/*.py:Use@st.fragmentdecorator for interactive UI updates without full page reloads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@content/normalization.py` around lines 79 - 176, The normalization widget workflow in the current page-scope block should be moved into an `@st.fragment` so interactions only rerun that UI segment. Wrap the UI and button-driven pipeline logic around the selectboxes, st.text_input, and the "Apply Normalization Pipelines" flow in a fragment function, then call that fragment from the page body so the rest of the page is not re-executed on every widget change.Source: Coding guidelines
content/statistical.py (1)
85-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the statistical controls/results into an
@st.fragment.This page adds interactive analysis widgets without fragment scoping, so the entire page reruns on every control change. As per coding guidelines,
**/*.py:Use@st.fragmentdecorator for interactive UI updates without full page reloads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@content/statistical.py` around lines 85 - 158, The statistical controls and results block in the main Streamlit flow reruns the entire page on every widget change; move the interactive UI and execution logic into an `@st.fragment` so updates are scoped locally. Wrap the sections that build metadata_pl, render the selectboxes, and handle the Run Statistical Analysis button/results inside a fragment function, keeping the existing calculate_statistical_tests and adjust_fdr_lazy calls together there.Source: Coding guidelines
content/filtering.py (1)
58-163: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove the interactive filter block into an
@st.fragment.The controls and result view are rendered at module scope, so every widget interaction reruns the whole page instead of just this analysis section. As per coding guidelines,
**/*.py:Use@st.fragmentdecorator for interactive UI updates without full page reloads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@content/filtering.py` around lines 58 - 163, The filter UI and result rendering in the filtering section are still running at module scope, so every widget change reruns the whole page; wrap this interactive block in an `@st.fragment` to keep updates scoped. Move the controls, Apply Filter button, and filtered result display into a fragment function in content/filtering.py, and keep the existing filter_method, filter_low_abundance, filter_low_repeatability, filter_low_variance, and filtered_df logic inside that fragment so the behavior stays the same but only this section refreshes.Source: Coding guidelines
content/enrichment.py (1)
50-140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse an
@st.fragmentfor the enrichment controls and result tabs.This page is another interactive analysis surface implemented at top level, so the whole page reruns on every control change. As per coding guidelines,
**/*.py:Use@st.fragmentdecorator for interactive UI updates without full page reloads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@content/enrichment.py` around lines 50 - 140, The enrichment controls and result tabs are implemented directly in the top-level Streamlit flow, causing full-page reruns on every widget change. Refactor the UI block around the threshold inputs, the run button, and the tabbed GO results into a dedicated function and decorate it with `@st.fragment` so only that section updates interactively. Keep the existing logic in calculate_go_enrichment, the target_p_col/p_label setup, and the tabs rendering inside the fragment.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@content/enrichment.py`:
- Around line 38-39: The fallback navigation in the `st.page_link` call
currently points to the wrong page module, which can break the prerequisite flow
if that target does not exist. Update the link in `content/enrichment.py` to use
the same registered statistics page path that `app.py` registers for the
statistics screen, keeping the label and icon unchanged.
- Around line 45-54: The enrichment setup currently validates only ProteinName,
but the later call path also depends on log2FC and a valid p-value column. In
content/enrichment.py, extend the existing preflight checks near target_p_col
selection to verify final_statistics_report contains log2FC plus either p-adj or
p-value before reaching calculate_go_enrichment. If any required column is
missing, show a clear st.error and st.stop early so the page fails fast with a
useful message.
In `@content/filtering.py`:
- Around line 142-145: The filter update in filtering.py should also invalidate
dependent analysis state so downstream pages stop using stale data. In the
filtering flow where filtered_df is written in the filtering logic, clear or
remove the existing imputed_df, normalized_df, and statistics_df entries from
st.session_state whenever the filter result changes. Make the change in the same
place that updates filtered_df so normalization and statistical paths that read
from imputed_df/normalized_df in content/normalization.py and
content/statistical.py will recompute from the new filtered data instead of
reusing old cached values.
In `@content/imputation.py`:
- Around line 124-128: After re-imputing in the imputation flow, clear any
downstream derived session-state artifacts that depend on the old data, not just
overwrite imputed_df. Update the logic around imputed_lazy.collect().to_pandas()
and the st.session_state assignments so normalized_df and statistics_df are
removed or reset whenever imputed_df is refreshed, ensuring later steps like
statistical and enrichment processing do not consume stale outputs.
In `@content/normalization.py`:
- Around line 166-170: The normalization step updates normalized_df but leaves
any existing statistics_df in session state, so downstream enrichment can use
stale statistics after a new normalization run. Update the normalization flow in
the final checkpointing block that sets st.session_state["normalized_df"] to
also clear or invalidate st.session_state["statistics_df"] whenever the
normalized data changes, so content/enrichment.py will not reuse mismatched
statistics.
In `@content/statistical.py`:
- Around line 103-157: The `anova` option in the `selected_method` flow is
incompatible with the downstream `log2FC`-based schema used after
`calculate_statistical_tests` and by consumers like `content/enrichment.py`.
Update the 3+ group branch in `content/statistical.py` so `method_options` only
includes methods that still produce a single per-row `log2FC`/p-value contract,
or change the downstream contract and documentation if ANOVA must remain. Make
sure the selected method path, result table messaging, and any dependent
filtering logic stay consistent with the same output columns.
- Around line 100-115: The statistical test selector in the group_count == 2
branch currently exposes "paired" even though no pair metadata is captured, so
remove that option from method_options in the selectbox flow and keep only
unpaired methods like "limma_like" and "welch"; update any related help_text or
validation in content/statistical.py so the UI no longer suggests paired
analysis until explicit pair assignment support exists.
In `@src/common/results_helpers.py`:
- Around line 234-246: The sample grouping logic in the results helper is
truncating to only the first two groups, which drops any additional groups
before downstream processing. Update the grouping and sample selection in the
results-building flow around the unique symbols df, sample_group_df,
group1_samples, group2_samples, and all_samples so it preserves every group
present in the data instead of hard-coding groups[0] and groups[1]. Make sure
the pivot/multi-group path receives the full set of samples and groups, with no
silent filtering when more than two groups exist.
---
Nitpick comments:
In `@content/enrichment.py`:
- Around line 50-140: The enrichment controls and result tabs are implemented
directly in the top-level Streamlit flow, causing full-page reruns on every
widget change. Refactor the UI block around the threshold inputs, the run
button, and the tabbed GO results into a dedicated function and decorate it with
`@st.fragment` so only that section updates interactively. Keep the existing logic
in calculate_go_enrichment, the target_p_col/p_label setup, and the tabs
rendering inside the fragment.
In `@content/filtering.py`:
- Around line 58-163: The filter UI and result rendering in the filtering
section are still running at module scope, so every widget change reruns the
whole page; wrap this interactive block in an `@st.fragment` to keep updates
scoped. Move the controls, Apply Filter button, and filtered result display into
a fragment function in content/filtering.py, and keep the existing
filter_method, filter_low_abundance, filter_low_repeatability,
filter_low_variance, and filtered_df logic inside that fragment so the behavior
stays the same but only this section refreshes.
In `@content/imputation.py`:
- Around line 65-134: The imputation controls/results block in
content/imputation.py should be scoped into a Streamlit fragment so widget
changes and the Apply Imputation button do not rerun the whole page. Wrap the
interactive section that builds metadata_pl, renders the selectbox/radio
controls, and handles the imputation execution around impute_mar and
impute_smallest_value with an `@st.fragment-decorated` function, then call that
fragment from the page so the UI updates stay localized.
In `@content/normalization.py`:
- Around line 79-176: The normalization widget workflow in the current
page-scope block should be moved into an `@st.fragment` so interactions only rerun
that UI segment. Wrap the UI and button-driven pipeline logic around the
selectboxes, st.text_input, and the "Apply Normalization Pipelines" flow in a
fragment function, then call that fragment from the page body so the rest of the
page is not re-executed on every widget change.
In `@content/statistical.py`:
- Around line 85-158: The statistical controls and results block in the main
Streamlit flow reruns the entire page on every widget change; move the
interactive UI and execution logic into an `@st.fragment` so updates are scoped
locally. Wrap the sections that build metadata_pl, render the selectboxes, and
handle the Run Statistical Analysis button/results inside a fragment function,
keeping the existing calculate_statistical_tests and adjust_fdr_lazy calls
together there.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3f986db5-e0e1-48bf-91e8-54d28dc2a8c8
📒 Files selected for processing (8)
app.pycontent/enrichment.pycontent/filtering.pycontent/imputation.pycontent/normalization.pycontent/results_abundance.pycontent/statistical.pysrc/common/results_helpers.py
| st.page_link( | ||
| "content/results_statistics.py", label="Go to Statistical Inference", icon="🔬" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Point this fallback link at the registered statistics page.
app.py registers content/statistical.py, but this button sends users to content/results_statistics.py. If that file is absent, the prerequisite flow dead-ends here.
Proposed fix
st.page_link(
- "content/results_statistics.py", label="Go to Statistical Inference", icon="🔬"
+ "content/statistical.py", label="Go to Statistical Inference", icon="🔬"
)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| st.page_link( | |
| "content/results_statistics.py", label="Go to Statistical Inference", icon="🔬" | |
| st.page_link( | |
| "content/statistical.py", label="Go to Statistical Inference", icon="🔬" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@content/enrichment.py` around lines 38 - 39, The fallback navigation in the
`st.page_link` call currently points to the wrong page module, which can break
the prerequisite flow if that target does not exist. Update the link in
`content/enrichment.py` to use the same registered statistics page path that
`app.py` registers for the statistics screen, keeping the label and icon
unchanged.
| id_col = "ProteinName" | ||
| if id_col not in final_statistics_report.columns: | ||
| st.error(f"❌ Structural Error: Column '{id_col}' is missing from the active matrix context.") | ||
| st.stop() | ||
|
|
||
| # --- SECTION 1: Parameter Setup & Dynamic Cutoff Labels --- | ||
| st.subheader("Configure Enrichment Thresholds") | ||
|
|
||
| # Check if target p-value should be adjusted or raw based on previous selections (Fallback safely to 'p-adj') | ||
| target_p_col = "p-adj" if "p-adj" in final_statistics_report.columns else "p-value" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Validate every column the enrichment call depends on.
This only checks ProteinName, but the execution path also requires log2FC and one of p-adj / p-value. Right now the page can pass an incompatible statistics table into calculate_go_enrichment and fail much later with a less useful error.
Proposed fix
-# Identify target identifier columns dynamically
-id_col = "ProteinName"
-if id_col not in final_statistics_report.columns:
- st.error(f"❌ Structural Error: Column '{id_col}' is missing from the active matrix context.")
- st.stop()
+required_cols = {"ProteinName", "log2FC"}
+missing_cols = required_cols - set(final_statistics_report.columns)
+if missing_cols:
+ st.error(
+ f"❌ Structural Error: Missing required columns: {', '.join(sorted(missing_cols))}."
+ )
+ st.stop()
+
+id_col = "ProteinName"
# --- SECTION 1: Parameter Setup & Dynamic Cutoff Labels ---
st.subheader("Configure Enrichment Thresholds")
-# Check if target p-value should be adjusted or raw based on previous selections (Fallback safely to 'p-adj')
-target_p_col = "p-adj" if "p-adj" in final_statistics_report.columns else "p-value"
+if "p-adj" in final_statistics_report.columns:
+ target_p_col = "p-adj"
+elif "p-value" in final_statistics_report.columns:
+ target_p_col = "p-value"
+else:
+ st.error("❌ Structural Error: Missing both 'p-adj' and 'p-value'.")
+ st.stop()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| id_col = "ProteinName" | |
| if id_col not in final_statistics_report.columns: | |
| st.error(f"❌ Structural Error: Column '{id_col}' is missing from the active matrix context.") | |
| st.stop() | |
| # --- SECTION 1: Parameter Setup & Dynamic Cutoff Labels --- | |
| st.subheader("Configure Enrichment Thresholds") | |
| # Check if target p-value should be adjusted or raw based on previous selections (Fallback safely to 'p-adj') | |
| target_p_col = "p-adj" if "p-adj" in final_statistics_report.columns else "p-value" | |
| required_cols = {"ProteinName", "log2FC"} | |
| missing_cols = required_cols - set(final_statistics_report.columns) | |
| if missing_cols: | |
| st.error( | |
| f"❌ Structural Error: Missing required columns: {', '.join(sorted(missing_cols))}." | |
| ) | |
| st.stop() | |
| id_col = "ProteinName" | |
| # --- SECTION 1: Parameter Setup & Dynamic Cutoff Labels --- | |
| st.subheader("Configure Enrichment Thresholds") | |
| if "p-adj" in final_statistics_report.columns: | |
| target_p_col = "p-adj" | |
| elif "p-value" in final_statistics_report.columns: | |
| target_p_col = "p-value" | |
| else: | |
| st.error("❌ Structural Error: Missing both 'p-adj' and 'p-value'.") | |
| st.stop() |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@content/enrichment.py` around lines 45 - 54, The enrichment setup currently
validates only ProteinName, but the later call path also depends on log2FC and a
valid p-value column. In content/enrichment.py, extend the existing preflight
checks near target_p_col selection to verify final_statistics_report contains
log2FC plus either p-adj or p-value before reaching calculate_go_enrichment. If
any required column is missing, show a clear st.error and st.stop early so the
page fails fast with a useful message.
| # Collect the evaluated lazy graph and convert back to Pandas for visualization | ||
| filtered_df = filtered_lazy.collect().to_pandas() | ||
| st.session_state["filtered_df"] = filtered_df | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Invalidate downstream analysis state when the filter output changes.
After Line 144 writes a new filtered_df, the old imputed_df, normalized_df, and statistics_df remain live. content/normalization.py prefers imputed_df over filtered_df, and content/statistical.py prefers normalized_df/imputed_df, so later pages can keep running on stale pre-filter data.
Proposed fix
# Collect the evaluated lazy graph and convert back to Pandas for visualization
filtered_df = filtered_lazy.collect().to_pandas()
+ for key in ("imputed_df", "normalized_df", "statistics_df"):
+ st.session_state.pop(key, None)
st.session_state["filtered_df"] = filtered_df📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Collect the evaluated lazy graph and convert back to Pandas for visualization | |
| filtered_df = filtered_lazy.collect().to_pandas() | |
| st.session_state["filtered_df"] = filtered_df | |
| # Collect the evaluated lazy graph and convert back to Pandas for visualization | |
| filtered_df = filtered_lazy.collect().to_pandas() | |
| for key in ("imputed_df", "normalized_df", "statistics_df"): | |
| st.session_state.pop(key, None) | |
| st.session_state["filtered_df"] = filtered_df |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@content/filtering.py` around lines 142 - 145, The filter update in
filtering.py should also invalidate dependent analysis state so downstream pages
stop using stale data. In the filtering flow where filtered_df is written in the
filtering logic, clear or remove the existing imputed_df, normalized_df, and
statistics_df entries from st.session_state whenever the filter result changes.
Make the change in the same place that updates filtered_df so normalization and
statistical paths that read from imputed_df/normalized_df in
content/normalization.py and content/statistical.py will recompute from the new
filtered data instead of reusing old cached values.
| # Resolve lazy graph optimization tree and push to display data frame structure | ||
| imputed_df = imputed_lazy.collect().to_pandas() | ||
|
|
||
| # 💾 Save current output into Session State for down-stream processing (Normalization, Statistics) | ||
| st.session_state["imputed_df"] = imputed_df |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Clear derived normalization/statistics outputs after re-imputation.
When Line 128 overwrites imputed_df, the old normalized_df and statistics_df are left intact. content/statistical.py will keep preferring normalized_df, and content/enrichment.py can still read stale statistics_df, so the pipeline becomes internally inconsistent.
Proposed fix
# Resolve lazy graph optimization tree and push to display data frame structure
imputed_df = imputed_lazy.collect().to_pandas()
# 💾 Save current output into Session State for down-stream processing (Normalization, Statistics)
+ for key in ("normalized_df", "statistics_df"):
+ st.session_state.pop(key, None)
st.session_state["imputed_df"] = imputed_df📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Resolve lazy graph optimization tree and push to display data frame structure | |
| imputed_df = imputed_lazy.collect().to_pandas() | |
| # 💾 Save current output into Session State for down-stream processing (Normalization, Statistics) | |
| st.session_state["imputed_df"] = imputed_df | |
| # Resolve lazy graph optimization tree and push to display data frame structure | |
| imputed_df = imputed_lazy.collect().to_pandas() | |
| # 💾 Save current output into Session State for down-stream processing (Normalization, Statistics) | |
| for key in ("normalized_df", "statistics_df"): | |
| st.session_state.pop(key, None) | |
| st.session_state["imputed_df"] = imputed_df |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@content/imputation.py` around lines 124 - 128, After re-imputing in the
imputation flow, clear any downstream derived session-state artifacts that
depend on the old data, not just overwrite imputed_df. Update the logic around
imputed_lazy.collect().to_pandas() and the st.session_state assignments so
normalized_df and statistics_df are removed or reset whenever imputed_df is
refreshed, ensuring later steps like statistical and enrichment processing do
not consume stale outputs.
| # Finalize and collect pipeline query graph optimizations | ||
| normalized_df = processing_lazy.collect().to_pandas() | ||
|
|
||
| # 💾 Save processing checkpoint inside Session State for Downstream (Statistics Block) | ||
| st.session_state["normalized_df"] = normalized_df |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Invalidate stale statistics after normalization changes.
Line 170 updates normalized_df, but any existing statistics_df survives. Since content/enrichment.py reads statistics_df directly from session state, users can see enrichment results for a different normalization run until they manually rerun statistics.
Proposed fix
# Finalize and collect pipeline query graph optimizations
normalized_df = processing_lazy.collect().to_pandas()
# 💾 Save processing checkpoint inside Session State for Downstream (Statistics Block)
+ st.session_state.pop("statistics_df", None)
st.session_state["normalized_df"] = normalized_df📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Finalize and collect pipeline query graph optimizations | |
| normalized_df = processing_lazy.collect().to_pandas() | |
| # 💾 Save processing checkpoint inside Session State for Downstream (Statistics Block) | |
| st.session_state["normalized_df"] = normalized_df | |
| # Finalize and collect pipeline query graph optimizations | |
| normalized_df = processing_lazy.collect().to_pandas() | |
| # 💾 Save processing checkpoint inside Session State for Downstream (Statistics Block) | |
| st.session_state.pop("statistics_df", None) | |
| st.session_state["normalized_df"] = normalized_df |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@content/normalization.py` around lines 166 - 170, The normalization step
updates normalized_df but leaves any existing statistics_df in session state, so
downstream enrichment can use stale statistics after a new normalization run.
Update the normalization flow in the final checkpointing block that sets
st.session_state["normalized_df"] to also clear or invalidate
st.session_state["statistics_df"] whenever the normalized data changes, so
content/enrichment.py will not reuse mismatched statistics.
| if group_count == 2: | ||
| method_options = ["limma_like", "welch", "paired"] | ||
| help_text = "'limma_like' uses Empirical Bayes variance shrinking. 'welch' is for unequal variances. 'paired' is for dependent samples." | ||
| elif group_count >= 3: | ||
| method_options = ["limma_like", "anova"] | ||
| help_text = "'limma_like' supports multi-group design matrices. 'anova' computes standard row-wise One-way ANOVA." | ||
| else: | ||
| st.error("❌ Statistical testing requires at least 2 unique sample groups.") | ||
| st.stop() | ||
|
|
||
| selected_method = st.selectbox( | ||
| "Select Statistical Test", | ||
| options=method_options, | ||
| index=0, | ||
| help=help_text | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Remove the paired-test option until pair metadata exists.
Nothing in this flow captures pair assignments, so paired can only pair samples by incidental column order. That makes the result statistically wrong for ordinary unpaired cohorts.
Proposed fix
if group_count == 2:
- method_options = ["limma_like", "welch", "paired"]
- help_text = "'limma_like' uses Empirical Bayes variance shrinking. 'welch' is for unequal variances. 'paired' is for dependent samples."
+ method_options = ["limma_like", "welch"]
+ help_text = "'limma_like' uses Empirical Bayes variance shrinking. 'welch' is for unequal variances."📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if group_count == 2: | |
| method_options = ["limma_like", "welch", "paired"] | |
| help_text = "'limma_like' uses Empirical Bayes variance shrinking. 'welch' is for unequal variances. 'paired' is for dependent samples." | |
| elif group_count >= 3: | |
| method_options = ["limma_like", "anova"] | |
| help_text = "'limma_like' supports multi-group design matrices. 'anova' computes standard row-wise One-way ANOVA." | |
| else: | |
| st.error("❌ Statistical testing requires at least 2 unique sample groups.") | |
| st.stop() | |
| selected_method = st.selectbox( | |
| "Select Statistical Test", | |
| options=method_options, | |
| index=0, | |
| help=help_text | |
| ) | |
| if group_count == 2: | |
| method_options = ["limma_like", "welch"] | |
| help_text = "'limma_like' uses Empirical Bayes variance shrinking. 'welch' is for unequal variances." | |
| elif group_count >= 3: | |
| method_options = ["limma_like", "anova"] | |
| help_text = "'limma_like' supports multi-group design matrices. 'anova' computes standard row-wise One-way ANOVA." | |
| else: | |
| st.error("❌ Statistical testing requires at least 2 unique sample groups.") | |
| st.stop() | |
| selected_method = st.selectbox( | |
| "Select Statistical Test", | |
| options=method_options, | |
| index=0, | |
| help=help_text | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@content/statistical.py` around lines 100 - 115, The statistical test selector
in the group_count == 2 branch currently exposes "paired" even though no pair
metadata is captured, so remove that option from method_options in the selectbox
flow and keep only unpaired methods like "limma_like" and "welch"; update any
related help_text or validation in content/statistical.py so the UI no longer
suggests paired analysis until explicit pair assignment support exists.
| elif group_count >= 3: | ||
| method_options = ["limma_like", "anova"] | ||
| help_text = "'limma_like' supports multi-group design matrices. 'anova' computes standard row-wise One-way ANOVA." | ||
| else: | ||
| st.error("❌ Statistical testing requires at least 2 unique sample groups.") | ||
| st.stop() | ||
|
|
||
| selected_method = st.selectbox( | ||
| "Select Statistical Test", | ||
| options=method_options, | ||
| index=0, | ||
| help=help_text | ||
| ) | ||
|
|
||
| with col2: | ||
| st.markdown("### 🛡️ 2. Multiple Testing Correction (FDR)") | ||
| selected_fdr = st.selectbox( | ||
| "Select FDR Adjustment Strategy", | ||
| options=["BH", "Bonferroni", "None"], | ||
| index=0, | ||
| help="'BH' (Benjamini-Hochberg) controls False Discovery Rate. 'Bonferroni' is strict Family-Wise Error Rate control." | ||
| ) | ||
|
|
||
| # --- SECTION 3: Statistical Query Execution --- | ||
| st.markdown("<br>", unsafe_allow_html=True) | ||
| if st.button("Run Statistical Analysis", type="primary"): | ||
|
|
||
| # Convert active pandas dataframe into polars lazyframe graph | ||
| stats_lazy = pl.from_pandas(base_df).lazy() | ||
|
|
||
| try: | ||
| # Execute Chain 1: Calculate core statistics (Adds log2FC, stat, p-value) | ||
| stats_lazy = calculate_statistical_tests( | ||
| quantification_data=stats_lazy, | ||
| metadata=metadata_pl, | ||
| method=selected_method | ||
| ) | ||
|
|
||
| # Execute Chain 2: Adjust Multiple Testing (Adds p-adj) | ||
| stats_lazy = adjust_fdr_lazy( | ||
| quantification_data=stats_lazy, | ||
| strategy=selected_fdr | ||
| ) | ||
|
|
||
| # Resolve lazy graph optimization tree and bring back to pandas memory | ||
| statistics_df = stats_lazy.collect().to_pandas() | ||
|
|
||
| # 💾 Save processing checkpoint inside Session State for Downstream (e.g., Volcano plot, Volcano/Heatmap UI) | ||
| st.session_state["statistics_df"] = statistics_df | ||
|
|
||
| st.success(f"Successfully calculated **{selected_method}** test with **{selected_fdr}** FDR correction!") | ||
|
|
||
| # Display the finalized statistics table view | ||
| st.subheader("Statistical Analysis Results") | ||
| st.markdown(f"Generated framework containing columns: `ProteinName`, `log2FC`, `stat`, `p-value`, `p-adj`, `PeptideSequence`") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
The ANOVA path does not match the downstream log2FC contract.
For 3+ groups, this page allows anova, but Line 157 documents a log2FC output and content/enrichment.py filters on |log2FC|. A one-way ANOVA does not produce a single fold-change per protein, so this branch is incompatible with the current downstream schema.
🧰 Tools
🪛 Ruff (0.15.18)
[error] 157-157: f-string without any placeholders
Remove extraneous f prefix
(F541)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@content/statistical.py` around lines 103 - 157, The `anova` option in the
`selected_method` flow is incompatible with the downstream `log2FC`-based schema
used after `calculate_statistical_tests` and by consumers like
`content/enrichment.py`. Update the 3+ group branch in `content/statistical.py`
so `method_options` only includes methods that still produce a single per-row
`log2FC`/p-value contract, or change the downstream contract and documentation
if ANOVA must remain. Make sure the selected method path, result table
messaging, and any dependent filtering logic stay consistent with the same
output columns.
| groups = sorted(df["Group"].unique()) | ||
|
|
||
| if len(groups) < 2: | ||
| return None | ||
|
|
||
| group1, group2 = groups[:2] | ||
|
|
||
| # Compute statistics per protein | ||
| stats_rows = [] | ||
| for protein, protein_df in df.groupby("ProteinName"): | ||
| g1_vals = protein_df[protein_df["Group"] == group1]["Intensity"].values | ||
| g2_vals = protein_df[protein_df["Group"] == group2]["Intensity"].values | ||
|
|
||
| if len(g1_vals) < 2 or len(g2_vals) < 2: | ||
| pval = np.nan | ||
| else: | ||
| _, pval = ttest_ind(g1_vals, g2_vals, equal_var=False) | ||
|
|
||
| mean_g1 = np.mean(g1_vals) if len(g1_vals) > 0 else np.nan | ||
| mean_g2 = np.mean(g2_vals) if len(g2_vals) > 0 else np.nan | ||
|
|
||
| log2fc = np.log2(mean_g2 / mean_g1) if mean_g1 > 0 else np.nan | ||
|
|
||
| stats_rows.append({ | ||
| "ProteinName": protein, | ||
| "log2FC": log2fc, | ||
| "p-value": pval, | ||
| }) | ||
|
|
||
| stats_df = pd.DataFrame(stats_rows) | ||
|
|
||
| if not stats_df.empty: | ||
| mask = stats_df["p-value"].notna() | ||
| if mask.any(): | ||
| _, p_adj, _, _ = multipletests(stats_df.loc[mask, "p-value"], method="fdr_bh") | ||
| stats_df.loc[mask, "p-adj"] = p_adj | ||
| else: | ||
| stats_df["p-adj"] = np.nan | ||
|
|
||
| # Order samples by group (group2 first, then group1) | ||
| # 3. Define ordering and build sample arrays | ||
| sample_group_df = df[["Sample", "Group"]].drop_duplicates() | ||
| group2_samples = sample_group_df[sample_group_df["Group"] == group2]["Sample"].tolist() | ||
| group1_samples = sample_group_df[sample_group_df["Group"] == group1]["Sample"].tolist() | ||
| all_samples = group2_samples + group1_samples | ||
|
|
||
| # Build pivot table | ||
| group1_samples = sample_group_df[sample_group_df["Group"] == groups[0]][ | ||
| "Sample" | ||
| ].tolist() | ||
| group2_samples = sample_group_df[sample_group_df["Group"] == groups[1]][ | ||
| "Sample" | ||
| ].tolist() | ||
| all_samples = group1_samples + group2_samples |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Don't truncate the pivot table to the first two groups.
all_samples is built from groups[0] and groups[1] only, so any third-or-later group is dropped from pivot_df. That silently removes data before it reaches the multi-group paths introduced in this PR.
Proposed fix
- groups = sorted(df["Group"].unique())
+ groups = sorted(df["Group"].unique())
if len(groups) < 2:
return None
# 3. Define ordering and build sample arrays
sample_group_df = df[["Sample", "Group"]].drop_duplicates()
- group1_samples = sample_group_df[sample_group_df["Group"] == groups[0]][
- "Sample"
- ].tolist()
- group2_samples = sample_group_df[sample_group_df["Group"] == groups[1]][
- "Sample"
- ].tolist()
- all_samples = group1_samples + group2_samples
+ all_samples = []
+ for group in groups:
+ all_samples.extend(
+ sample_group_df.loc[sample_group_df["Group"] == group, "Sample"].tolist()
+ )📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| groups = sorted(df["Group"].unique()) | |
| if len(groups) < 2: | |
| return None | |
| group1, group2 = groups[:2] | |
| # Compute statistics per protein | |
| stats_rows = [] | |
| for protein, protein_df in df.groupby("ProteinName"): | |
| g1_vals = protein_df[protein_df["Group"] == group1]["Intensity"].values | |
| g2_vals = protein_df[protein_df["Group"] == group2]["Intensity"].values | |
| if len(g1_vals) < 2 or len(g2_vals) < 2: | |
| pval = np.nan | |
| else: | |
| _, pval = ttest_ind(g1_vals, g2_vals, equal_var=False) | |
| mean_g1 = np.mean(g1_vals) if len(g1_vals) > 0 else np.nan | |
| mean_g2 = np.mean(g2_vals) if len(g2_vals) > 0 else np.nan | |
| log2fc = np.log2(mean_g2 / mean_g1) if mean_g1 > 0 else np.nan | |
| stats_rows.append({ | |
| "ProteinName": protein, | |
| "log2FC": log2fc, | |
| "p-value": pval, | |
| }) | |
| stats_df = pd.DataFrame(stats_rows) | |
| if not stats_df.empty: | |
| mask = stats_df["p-value"].notna() | |
| if mask.any(): | |
| _, p_adj, _, _ = multipletests(stats_df.loc[mask, "p-value"], method="fdr_bh") | |
| stats_df.loc[mask, "p-adj"] = p_adj | |
| else: | |
| stats_df["p-adj"] = np.nan | |
| # Order samples by group (group2 first, then group1) | |
| # 3. Define ordering and build sample arrays | |
| sample_group_df = df[["Sample", "Group"]].drop_duplicates() | |
| group2_samples = sample_group_df[sample_group_df["Group"] == group2]["Sample"].tolist() | |
| group1_samples = sample_group_df[sample_group_df["Group"] == group1]["Sample"].tolist() | |
| all_samples = group2_samples + group1_samples | |
| # Build pivot table | |
| group1_samples = sample_group_df[sample_group_df["Group"] == groups[0]][ | |
| "Sample" | |
| ].tolist() | |
| group2_samples = sample_group_df[sample_group_df["Group"] == groups[1]][ | |
| "Sample" | |
| ].tolist() | |
| all_samples = group1_samples + group2_samples | |
| groups = sorted(df["Group"].unique()) | |
| if len(groups) < 2: | |
| return None | |
| # 3. Define ordering and build sample arrays | |
| sample_group_df = df[["Sample", "Group"]].drop_duplicates() | |
| all_samples = [] | |
| for group in groups: | |
| all_samples.extend( | |
| sample_group_df.loc[sample_group_df["Group"] == group, "Sample"].tolist() | |
| ) |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/common/results_helpers.py` around lines 234 - 246, The sample grouping
logic in the results helper is truncating to only the first two groups, which
drops any additional groups before downstream processing. Update the grouping
and sample selection in the results-building flow around the unique symbols df,
sample_group_df, group1_samples, group2_samples, and all_samples so it preserves
every group present in the data instead of hard-coding groups[0] and groups[1].
Make sure the pivot/multi-group path receives the full set of samples and
groups, with no silent filtering when more than two groups exist.
There was a problem hiding this comment.
🧹 Nitpick comments (4)
requirements.txt (1)
152-153: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDrop the duplicate
polarsrequirement.
polarsis already pinned with a floor at Line 144. Keeping a second bare entry here is redundant and makes future version updates easier to miss.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@requirements.txt` around lines 152 - 153, Remove the redundant bare polars entry from the requirements list, since polars is already specified earlier with a version floor. Keep the pinned occurrence only and update the dependency block so the requirements remain singular and easier to maintain.content/results_volcano.py (2)
44-84: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winWrap the threshold controls and render path in
@st.fragment.Both sliders currently rerun the full page and rebuild the component on every change. A fragment keeps those interactions scoped to the plot.
As per coding guidelines,
**/*.py: Use@st.fragmentdecorator for interactive UI updates without full page reloads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@content/results_volcano.py` around lines 44 - 84, The threshold sliders and VolcanoPlot render path are triggering full page reruns instead of a scoped update. Move the UI block in results_volcano.py into a Streamlit fragment by wrapping the section that creates fc_thresh, p_thresh, VolcanoPlot, and the call through volcano_plot_component in a function decorated with `@st.fragment`, so slider changes only refresh the plot area and not the entire page.Source: Coding guidelines
61-73: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winSet an adjusted-p-value Y label explicitly.
This plot is driven by
p-adj, but the generatedquantms_volcano_plot/manifest.jsonstill labels the Y axis as-log10(p-value). That will misstate what the chart is showing unless you override the label here.Possible fix
volcano_plot_component = VolcanoPlot( cache_id="quantms_volcano_plot", data=volcano_pl_lazy, log2fc_column="log2FC", pvalue_column="p-adj", label_column="ProteinName", + y_label="-log10(adjusted p-value)", up_color="`#E74C3C`", down_color="`#3498DB`", ns_color="`#95A5A6`", show_threshold_lines=True, threshold_line_style="dash", )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@content/results_volcano.py` around lines 61 - 73, The VolcanoPlot setup is using p-adj as the p-value source but still leaves the generated Y-axis label misleading. Update the VolcanoPlot constructor in results_volcano.py to explicitly set the Y-axis label for adjusted p-values, using the component’s label override option rather than the default -log10(p-value) text. Keep the existing cache_id, pvalue_column, and other settings unchanged.content/results_heatmap.py (1)
45-97: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winWrap the slider-driven render path in
@st.fragment.Changing
top_nreruns the full page, including data prep and component setup. Moving this block into a fragment keeps the update local and matches the repo pattern.As per coding guidelines,
**/*.py: Use@st.fragmentdecorator for interactive UI updates without full page reloads.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@content/results_heatmap.py` around lines 45 - 97, The `top_n` slider-driven heatmap render path in `results_heatmap.py` is causing a full-page rerun instead of a local UI update. Wrap the block that builds `var_series`, `top_proteins`, `heatmap_z`, `melted_df`, and renders `Heatmap`/`state_manager` in an `@st.fragment`-decorated function so the `st.slider` interaction updates only that section. Keep the existing logic and symbols like `top_n`, `Heatmap`, and `state_manager`, but move them into the fragment to match the repo’s interactive update pattern.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@content/results_heatmap.py`:
- Around line 45-97: The `top_n` slider-driven heatmap render path in
`results_heatmap.py` is causing a full-page rerun instead of a local UI update.
Wrap the block that builds `var_series`, `top_proteins`, `heatmap_z`,
`melted_df`, and renders `Heatmap`/`state_manager` in an
`@st.fragment`-decorated function so the `st.slider` interaction updates only
that section. Keep the existing logic and symbols like `top_n`, `Heatmap`, and
`state_manager`, but move them into the fragment to match the repo’s interactive
update pattern.
In `@content/results_volcano.py`:
- Around line 44-84: The threshold sliders and VolcanoPlot render path are
triggering full page reruns instead of a scoped update. Move the UI block in
results_volcano.py into a Streamlit fragment by wrapping the section that
creates fc_thresh, p_thresh, VolcanoPlot, and the call through
volcano_plot_component in a function decorated with `@st.fragment`, so slider
changes only refresh the plot area and not the entire page.
- Around line 61-73: The VolcanoPlot setup is using p-adj as the p-value source
but still leaves the generated Y-axis label misleading. Update the VolcanoPlot
constructor in results_volcano.py to explicitly set the Y-axis label for
adjusted p-values, using the component’s label override option rather than the
default -log10(p-value) text. Keep the existing cache_id, pvalue_column, and
other settings unchanged.
In `@requirements.txt`:
- Around line 152-153: Remove the redundant bare polars entry from the
requirements list, since polars is already specified earlier with a version
floor. Keep the pinned occurrence only and update the dependency block so the
requirements remain singular and easier to maintain.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ffb4a024-5385-44da-b83e-14d74207c3d2
⛔ Files ignored due to path filters (3)
quantms_protein_heatmap/preprocessed/level_0.parquetis excluded by!**/*.parquetquantms_protein_heatmap/preprocessed/level_1.parquetis excluded by!**/*.parquetquantms_volcano_plot/preprocessed/volcanoData.parquetis excluded by!**/*.parquet
📒 Files selected for processing (8)
app.pycontent/results_heatmap.pycontent/results_pca.pycontent/results_volcano.pyquantms_protein_heatmap/manifest.jsonquantms_volcano_plot/manifest.jsonrequirements.txtsrc/common/results_helpers.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/common/results_helpers.py
|
@hjn0415a Could you resolve the conflicts and ping me? Then I will merge and redeploy. |
Streamlit was crashing with WebSocket "Connection error" on the Volcano/PCA/Heatmap pages because polars' default CPU-core-sized native thread pool (Rayon) access-violation crashed the whole process when invoked from Streamlit's script-runner thread on a high-core-count Windows machine. Force POLARS_MAX_THREADS=1 before polars is imported anywhere, and prefer polars' most CPU-compatible native runtime as a secondary safeguard. Also restores results_pca.py's in-progress migration to the openms-insight PCAPlot component, which had a corrupted trailing duplicate block from an earlier revert. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…onent Adds a scratch/test page (results_heatmap_clustered.py) wired into the navigation to visually verify openms-insight's new ClusteredHeatmap component (real grid heatmap with row/column dendrograms and a sample group color bar) against real workspace data, without touching the existing results_heatmap.py page. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Keeps the existing 2 colors (teal/salmon) unchanged for the 2-group case, and adds 8 more hue-spread colors so datasets with 3-10 sample groups each get a visually distinct color instead of cycling back to the first two. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The confidence ellipse for small sample groups (n=3-4) is statistically unstable and was dominating the plot's axis auto-range, so pass show_ellipses=False to PCAPlot in results_pca.py. quantms_pca_plot/, quantms_protein_heatmap/, and quantms_volcano_plot/ are runtime caches regenerated by openms-insight on every run (keyed by cache_id) and shouldn't be committed - untrack them and add to .gitignore. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@content/results_pca.py`:
- Around line 101-160: The PCA configuration and rendering around PCAPlot in
content/results_pca.py#L101-L160 must be moved into an `@st.fragment` function,
converted to the required Plotly visualization flow, and rendered through
streamlit_plotly_events while preserving component selection and state handling.
Apply the same fragment-based configuration/rendering and mandated Plotly plus
streamlit_plotly_events interaction flow to the heatmap section in
content/results_heatmap_clustered.py#L40-L100.
- Line 30: Update the get_abundance_data() unpacking in content/results_pca.py
at lines 30-30 and content/results_heatmap_clustered.py at lines 31-31 to accept
all three returned values, discarding the unused expr_df while retaining
pivot_df and group_map for rendering.
- Around line 162-165: Update the explained-variance pairing in the st.markdown
expression to use zip(strict=True) when combining pc_columns and variance_ratio,
preserving the existing label formatting while preventing silent truncation when
their lengths differ.
In `@requirements.txt`:
- Around line 145-149: Replace the standalone polars-runtime-compat dependency
with the supported Polars rtcompat extra in both dependency blocks: change each
base Polars declaration to include [rtcompat], preserving the existing version
constraint for polars>=1.0.0 and the unversioned online-mode polars entry.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 50664a59-0700-42c8-aad9-891b91738cf9
📒 Files selected for processing (5)
.gitignoreapp.pycontent/results_heatmap_clustered.pycontent/results_pca.pyrequirements.txt
🚧 Files skipped from review as they are similar to previous changes (1)
- app.py
| st.stop() | ||
|
|
||
| pivot_df, expr_df, group_map = result | ||
| pivot_df, group_map = result |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Unpack the full abundance-data contract.
get_abundance_data() returns (pivot_df, expr_df, group_map), so both pages currently raise ValueError: too many values to unpack before rendering.
content/results_pca.py#L30-L30: discard the unused expression table while retaining the pivot table and group map.content/results_heatmap_clustered.py#L31-L31: discard the unused expression table while retaining the pivot table and group map.
- pivot_df, group_map = result
+ pivot_df, _expr_df, group_map = result📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| pivot_df, group_map = result | |
| pivot_df, _expr_df, group_map = result |
📍 Affects 2 files
content/results_pca.py#L30-L30(this comment)content/results_heatmap_clustered.py#L31-L31
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@content/results_pca.py` at line 30, Update the get_abundance_data() unpacking
in content/results_pca.py at lines 30-30 and
content/results_heatmap_clustered.py at lines 31-31 to accept all three returned
values, discarding the unused expr_df while retaining pivot_df and group_map for
rendering.
| top_n = st.slider( | ||
| "Number of proteins (Highest Variance)", | ||
| min_value=20, | ||
| max_value=min(5000, max_available), | ||
| value=min(500, max_available), | ||
| step=10, | ||
| key="pca_top_n", | ||
| help=( | ||
| "PCA is computed only on the N proteins with the highest variance " | ||
| "across samples, to reduce noise from low-variance/uninformative features." | ||
| ), | ||
| ) | ||
|
|
||
| top_proteins = expr_df.var(axis=1).sort_values(ascending=False).head(top_n).index | ||
| expr_df_pca = expr_df.loc[top_proteins].reset_index() | ||
|
|
||
| if expr_df_pca.shape[0] < 2: | ||
| st.info("Not enough proteins after p-value filtering for PCA.") | ||
| st.info("Not enough proteins after variance filtering for PCA.") | ||
| st.stop() | ||
|
|
||
| X = expr_df_pca.T | ||
| X_scaled = StandardScaler().fit_transform(X) | ||
|
|
||
| pca = PCA(n_components=2) | ||
| pcs = pca.fit_transform(X_scaled) | ||
|
|
||
| pca_df = pd.DataFrame( | ||
| pcs, | ||
| columns=["PC1", "PC2"], | ||
| index=X.index | ||
| # Prepare structural Polars metadata DataFrame required by PCAPlot | ||
| metadata_pl = pl.DataFrame( | ||
| [{"sample_id": s, "group": group_map[s]} for s in sample_cols], | ||
| schema={"sample_id": pl.String, "group": pl.String}, | ||
| ) | ||
| pca_lazy = pl.from_pandas(expr_df_pca).lazy() | ||
|
|
||
| # 3. Initialize the OpenMS-Insight PCAPlot component (computes PCA internally) | ||
| try: | ||
| pca_component = PCAPlot( | ||
| cache_id="quantms_pca_plot", | ||
| data=pca_lazy, | ||
| metadata=metadata_pl, | ||
| sample_id_field="sample_id", | ||
| group_field="group", | ||
| n_components=5, | ||
| title="Sample PCA", | ||
| show_ellipses=False, | ||
| ) | ||
| except ValueError as e: | ||
| st.error(f"PCA computation failed: {e}") | ||
| st.stop() | ||
|
|
||
| norm_map = { | ||
| k.replace(".mzML", ""): v | ||
| for k, v in group_map.items() | ||
| } | ||
| pca_df["Group"] = pca_df.index.map(norm_map) | ||
|
|
||
| fig_pca = px.scatter( | ||
| pca_df, | ||
| x="PC1", | ||
| y="PC2", | ||
| color="Group", | ||
| text=pca_df.index, | ||
| ) | ||
| variance_ratio = pca_component.get_variance_ratio() | ||
| pc_columns = pca_component.get_pc_columns() | ||
|
|
||
| fig_pca.update_traces(textposition="top center") | ||
| fig_pca.update_layout( | ||
| xaxis_title=f"PC1 ({pca.explained_variance_ratio_[0]*100:.1f}%)", | ||
| yaxis_title=f"PC2 ({pca.explained_variance_ratio_[1]*100:.1f}%)", | ||
| height=600, | ||
| ) | ||
| # 4. Let the user pick which component pair to view (no recomputation needed) | ||
| col1, col2 = st.columns(2) | ||
| with col1: | ||
| pc_x_label = st.selectbox("X-axis component", pc_columns, index=0, key="pca_pc_x") | ||
| with col2: | ||
| default_y_index = 1 if len(pc_columns) > 1 else 0 | ||
| pc_y_label = st.selectbox("Y-axis component", pc_columns, index=default_y_index, key="pca_pc_y") | ||
|
|
||
| pc_x = int(pc_x_label.replace("PC", "")) | ||
| pc_y = int(pc_y_label.replace("PC", "")) | ||
|
|
||
| st.plotly_chart(fig_pca, use_container_width=True) | ||
| # 5. Render the component | ||
| state_manager = st.session_state.get("state") | ||
| pca_component(state_manager=state_manager, pc_x=pc_x, pc_y=pc_y, height=600) |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
Use the required fragment and Plotly interaction pattern.
The slider/selectbox changes rerun each complete page, and both visualizations bypass the required Plotly plus streamlit_plotly_events integration. Isolate the interactive sections in @st.fragment functions and render/capture interactions through the mandated Plotly event flow.
content/results_pca.py#L101-L160: fragment the PCA configuration/rendering and use the required Plotly event integration.content/results_heatmap_clustered.py#L40-L100: fragment the heatmap configuration/rendering and use the required Plotly event integration.
As per coding guidelines, use @st.fragment decorator for interactive UI updates without full page reloads and use Plotly and streamlit_plotly_events for interactive visualizations.
📍 Affects 2 files
content/results_pca.py#L101-L160(this comment)content/results_heatmap_clustered.py#L40-L100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@content/results_pca.py` around lines 101 - 160, The PCA configuration and
rendering around PCAPlot in content/results_pca.py#L101-L160 must be moved into
an `@st.fragment` function, converted to the required Plotly visualization flow,
and rendered through streamlit_plotly_events while preserving component
selection and state handling. Apply the same fragment-based
configuration/rendering and mandated Plotly plus streamlit_plotly_events
interaction flow to the heatmap section in
content/results_heatmap_clustered.py#L40-L100.
Source: Coding guidelines
| st.markdown( | ||
| "**Explained variance:** " | ||
| + ", ".join(f"{col} {ratio * 100:.1f}%" for col, ratio in zip(pc_columns, variance_ratio)) | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
fd -HI -t f \
-g 'pyproject.toml' -g 'setup.cfg' -g 'tox.ini' \
-g '.python-version' -g '.tool-versions' . \
-x sh -c 'echo "=== $1 ==="; rg -n "(requires-python|python_version|target-version|python)" "$1" || true' sh {}Repository: OpenMS/quantms-web
Length of output: 389
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
import json, subprocess, pathlib
targets = [
pathlib.Path("pyproject.toml"),
pathlib.Path("tox.ini"),
pathlib.Path(".python-version"),
pathlib.Path("pre-commit-config.yaml"),
pathlib.Path("requirements.txt"),
pathlib.Path("setup.py"),
]
print("tool/version/config files:")
for p in targets:
if p.exists():
print(f"=== {p} ===")
text = p.read_text(errors="replace")
for i, line in enumerate(text.splitlines(), 1):
if any(k in line.lower() for k in ["requires-python", "python_version", "target-version", "python", "python_requires", "3."]):
print(f"{i}: {line}")
# Locate PCA file and relevant lines without executing repo code.
pca = pathlib.Path("content/results_pca.py")
if pca.exists():
lines = pca.read_text(errors="replace").splitlines()
print("=== content/results_pca.py relevant lines 120-175 ===")
for i in range(120, 176):
if i <= len(lines):
print(f"{i:4}: {lines[i-1]}")
# Check local declarations for Ruff config target.
for p in pathlib.Path(".").rglob("ruff.toml"):
print(f"=== Ruff config {p} ===")
for i, line in enumerate(p.read_text(errors="replace").splitlines(), 1):
if "target-version" in line.lower() or "ruff" in line.lower():
print(f"{i}: {line}")
PY
# Inspect dependency files for declared Python compatibility.
printf '\n=== Python compatibility declarations ===\n'
git ls-files | rg '(^|/)(pyproject.toml|setup.py|setup.cfg|tox.ini|requirements[^/]*\.txt|Pipfile|environment.yml|\.python-version)$' || trueRepository: OpenMS/quantms-web
Length of output: 3015
🏁 Script executed:
#!/bin/bash
set -eu
# Check dependency/tooling files around Python, Streamlit, and ruff target versions without executing repository code.
for p in requirements.txt pyproject.toml tox.ini poetry.lock Pipfile.lock .pre-commit-config.yaml; do
if [ -f "$p" ]; then
echo "=== $p ==="
rg -n -i 'python|streamlit|ruff|strict|target-version|requires' "$p" || true
fi
done
# Resolve declared python-runtime markers in Lock metadata only if present and minimal.
python3 - <<'PY'
from pathlib import Path
for p in (Path("poetry.lock"), Path("Pipfile.lock"), Path("pyproject.toml"), Path("tox.ini")):
if not p.exists():
continue
text = p.read_text(errors="replace")
vals = {
"python-markers": [line.strip() for line in text.splitlines() if "Programming Language :: Python :: 3." in line or "Requires-Python" in line or "python_requires" in line.lower()],
}
vals["requires-python-matches-310+"] = any(("requires-python" in text.lower()) and
("3.10" in text.lower() or "3.11" in text.lower() or "3.12" in text.lower()))
print(f"=== {p} ===")
print(f"python markers: {vals['python-markers']}")
print("requires-python matches Python 3.10+: " + ("yes" if vals["requires-python-matches-310+"] else "no"))
PYRepository: OpenMS/quantms-web
Length of output: 866
Use zip(strict=True) for the explained-variance label/ratio pairing.
The dependency pin targets Python 3.12, so this pairwise-zip truncation guard can be added safely.
🧰 Tools
🪛 Ruff (0.15.21)
[warning] 164-164: zip() without an explicit strict= parameter
Add explicit value for parameter strict=
(B905)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@content/results_pca.py` around lines 162 - 165, Update the explained-variance
pairing in the st.markdown expression to use zip(strict=True) when combining
pc_columns and variance_ratio, preserving the existing label formatting while
preventing silent truncation when their lengths differ.
Source: Linters/SAST tools
| # Forces polars to prefer its most CPU-compatible native runtime. | ||
| # Without this, polars can select an AVX-optimized runtime (e.g. runtime-32) | ||
| # that access-violation crashes (0xC0000005 in _polars_runtime.pyd) on some | ||
| # CPUs (seen on AMD Threadripper PRO 3995WX on Windows) during dataframe ops. | ||
| polars-runtime-compat |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python -m pip install --dry-run --report /tmp/polars-resolution.json -r requirements.txt
python - <<'PY'
import json
with open("/tmp/polars-resolution.json") as f:
report = json.load(f)
for item in report.get("install", []):
name = item["metadata"]["name"].lower()
if name.startswith("polars"):
print(name, item["metadata"]["version"])
PYRepository: OpenMS/quantms-web
Length of output: 193
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== requirements excerpts =="
sed -n '135,165p' requirements.txt
echo
echo "== pip/tool availability =="
python3 - <<'PY'
import shutil, sys
for cmd in ("python3", "python3 -m pip", "pip", "uv"):
print(cmd, shutil.which(cmd.split()[0] if cmd.startswith("python3 -m") else cmd) or "not found")
PY
echo
echo "== pyproject/package references =="
rg -n "polars(\[|)|runtime-compat|rtcompat" -S . \
-g 'requirements*.txt' -g 'pyproject.toml' -g 'setup.*' -g 'uv.lock' -g 'poetry.lock' 2>/dev/null || trueRepository: OpenMS/quantms-web
Length of output: 1350
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== package metadata from pypi pypi_project api =="
python3 - <<'PY'
import json, urllib.request
for proj in ("polars", "polars-runtime-compat"):
url = f"https://pypi.org/pypi/{proj}/json"
print(f"--- {proj} ---")
with urllib.request.urlopen(url, timeout=20) as r:
data = json.load(r)
latest = data.get("info", {}).get("version")
print(f"latest: {latest}")
metadata = data.get("info", {}).get("requires_dist") or []
print("requires_dist:")
for req in metadata:
if "polars" in req.lower() or "runtime" in req.lower() or "rtcompat" in req.lower():
print(req)
print("all_requires_dist count:", len(metadata))
print("first 30 requirements:")
for req in metadata[:30]:
print(req)
PY
echo
echo "== exact resolver dependency links for current resolver =="
pip3 install --dry-run -r requirements.txt 2>&1 | rg -n "Collecting|polars|runtime-compat|Already satisfied|Will be installed|Requirement already" || trueRepository: OpenMS/quantms-web
Length of output: 2507
🌐 Web query:
PyPI polars runtime extra rtcompat polars-runtime-compat
💡 Result:
polars-runtime-compat is a specialized, dependency-only package containing the binary shared libraries required for the Polars Python library to function on specific hardware, particularly legacy CPUs [1][2]. Key details about polars-runtime-compat and the rtcompat extra: Purpose: It provides a build of Polars compiled without AVX (Advanced Vector Extensions) target features [3][4]. This is required for compatibility with older CPUs (e.g., those from before 2011) or when running x86-64 builds of Python on Apple Silicon via Rosetta [3][5]. Usage: Users should install it via the Polars extras syntax: pip install polars[rtcompat] [3][5] Architecture: It is one of several Polars runtime packages introduced to resolve issues with unpredictable shadowing of different build types [2]. The main Polars package functions by depending on these specific runtime packages [1][2]. These packages are intended for use as internal dependencies for the main polars package and are not meant for general, standalone consumption by end users [1][6]. Hierarchy: As of recent Polars releases, the runtime loading hierarchy ensures that Polars automatically and deterministically selects the appropriate runtime based on your system configuration [2]. polars-runtime-compat is designated as the most conservative runtime available in this hierarchy [2].
Citations:
- 1: https://pypi.org/project/polars-runtime-compat/
- 2: https://pola.rs/posts/polars-in-aggregate-dec25/
- 3: https://pypi.org/project/polars/
- 4: https://crates.io/crates/polars
- 5: https://docs.pola.rs/user-guide/installation/
- 6: https://pypi.org/project/polars-runtime-compat/1.39.2/
🌐 Web query:
Polars Python documentation install from source or source distribution with rtcompat extra
💡 Result:
The rtcompat extra is a configuration option provided in the Polars Python package to support legacy CPUs that lack certain instruction set features (such as AVX2) or to run on x86-64 builds of Python via Rosetta on Apple Silicon [1][2][3]. When installing Polars, you can use the rtcompat extra to install the polars-runtime-compat runtime [4]. To install using pip: pip install "polars[rtcompat]" To install using uv: uv pip install "polars[rtcompat]" or uv add polars --extra rtcompat [5] Regarding building from source: The rtcompat option primarily applies to pre-built wheels provided via PyPI [2][4]. When compiling Polars from source (e.g., using make commands like make build-release inside the py-polars directory), target CPU optimization is controlled by build flags rather than pip extras [1][6]. To build for an older CPU that does not support modern instruction sets like AVX2, you should specify the LTS_CPU environment variable when running your build command [1][7]: LTS_CPU=1 make build-release This instruction sets the compiler to avoid using modern CPU-specific target features, achieving an effect similar to the rtcompat runtime [1][6].
Citations:
- 1: https://github.com/pola-rs/polars/tree/main/py-polars
- 2: https://pypi.org/project/polars/
- 3: https://docs.rs/crate/polars/latest/source/README.md
- 4: https://pola.rs/posts/polars-in-aggregate-dec25/
- 5: Add uv installation instructions to the documentation pola-rs/polars#26558
- 6: https://github.com/pola-rs/polars
- 7: https://github.com/pola-rs/polars/blob/b6ae1153/README.md
🌐 Web query:
site:pola.rs docs user guide install polars rtcompat
💡 Result:
In Polars, the rtcompat extra is used to install a legacy version of the library that supports older CPUs lacking AVX2 support [1][2]. To install Polars with this compatibility runtime, you can use the following command: pip install "polars[rtcompat]" Alternatively, if you are using a package manager like uv, the syntax remains consistent: uv pip install "polars[rtcompat]" Background Polars has moved to a system of modular runtimes [2]. When you install the base polars package, it automatically selects the most appropriate runtime for your system [2]. The rtcompat extra specifically pulls in the polars-runtime-compat package (formerly known as polars-lts-cpu), which is designed for hardware that cannot run the standard, highly optimized versions of Polars [2]. Polars will automatically and deterministically load the most conservative runtime available on your system if multiple are installed [2].
Citations:
- 1: https://docs.pola.rs/user-guide/installation/
- 2: https://pola.rs/posts/polars-in-aggregate-dec25/
🌐 Web query:
pola.rs documentation runtime hierarchy polars multiple runtime packages
💡 Result:
Polars utilizes a multi-runtime architecture to optimize performance across different hardware configurations and data scale requirements while ensuring compatibility [1][2]. Instead of maintaining separate, potentially conflicting Python packages, Polars now uses a unified main package that dynamically selects the appropriate runtime binary at import time [1][3]. Runtime Hierarchy and Selection When Polars is imported, it automatically detects the system's capabilities and selects the most appropriate runtime variant [1][2]. The loading priority follows a deterministic hierarchy designed to balance performance and compatibility [1]: 1. polars-runtime-compat (Installed via polars[rtcompat]): The most conservative runtime, formerly known as polars-lts-cpu, designed for legacy CPUs with minimal feature requirements [1][2]. 2. polars-runtime-64 (Installed via polars[rt64]): A high-performance runtime for modern 64-bit CPUs, formerly known as polars-u64-index, which supports larger data structures [1][2]. 3. polars-runtime-32 (Default): The standard runtime included with the base polars installation, optimized for general modern x86-64 CPUs [1][2]. Mechanism The runtime selection is implemented in the internal _plr module, which validates CPU flags and package versions before loading the binary [4][2]. Users can influence or override this automated selection process using environment variables [2]: POLARS_FORCE_PKG: Forces the loader to use a specific runtime variant ("32", "64", or "compat"). It will raise an error if the requested variant is unavailable or version-mismatched [2]. POLARS_PREFER_PKG: Defines a preferred runtime to attempt before falling back to the standard search order [2]. Because these runtimes are now managed as secondary dependencies within a single package structure, this architecture eliminates the previous issue where installing different runtime variants could cause namespace conflicts and import errors [1][3]. Users are advised to avoid manual installation of separate legacy packages like polars-lts-cpu and instead use the provided extras, such as pip install "polars[rtcompat]" [1][3].
Citations:
- 1: https://pola.rs/posts/polars-in-aggregate-dec25/
- 2: https://deepwiki.com/pola-rs/polars/3.1-pyo3-bindings-and-ffi-layer
- 3: Installing
polarsandpolars-lts-cpuat different versions causesImportErrorsdue to mixed namespace files pola-rs/polars#26534 - 4: https://deepwiki.com/pola-rs/polars/3-python-rust-interface
Install the compatibility runtime through Polars’ rtcompat extra in both dependency blocks.
polars-runtime-compat is an internal runtime package; the supported installation path is polars[rtcompat], and Polars’ versioned runtime packages are not guaranteed to pair with standalone polars installs. requirements.txt still declares base polars>=1.0.0 and plain polars for online mode separately, so those installs can skip or mismatch the compatibility runtime and retain the default crashing runtime.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@requirements.txt` around lines 145 - 149, Replace the standalone
polars-runtime-compat dependency with the supported Polars rtcompat extra in
both dependency blocks: change each base Polars declaration to include
[rtcompat], preserving the existing version constraint for polars>=1.0.0 and the
unversioned online-mode polars entry.
2 participants
Implement 5 new Streamlit analysis pipeline pages powered by the openms-insight engine to handle downstream proteomics workflows:
Data Filtering (filtering.py):
Missing Value Imputation (imputation.py):
Data Normalization & Scaling (normalization.py):
Statistical Inference (statistical.py):
GO Enrichment Analysis (enrichment.py):
Summary by CodeRabbit
New Features
Updates
Bug Fixes