Conversation
…plot. Added threshold filtering.
There was a problem hiding this comment.
Pull request overview
This pull request implements a composite analysis feature that allows users to compare binding and perturbation datasets using multiple statistical methods (DTO, Rank Response P-value, Univariate P-value). The implementation includes dynamic boxplot visualization with threshold-based filtering to identify transcription factors that meet specified criteria across dataset combinations.
Changes:
- Added composite analysis visualization with dynamic faceted boxplots showing distributions across binding and perturbation source combinations
- Implemented threshold filtering with configurable operators (<, <=, >, >=) to identify TFs passing user-defined criteria
- Created reusable utilities for plot formatting and source name mapping
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
tfbpshiny/utils/rename_dataframe_data_sources.py |
Utility function for renaming data source columns (currently unused) |
tfbpshiny/utils/plot_formatter.py |
Shared Plotly formatting for consistent boxplot styling |
tfbpshiny/utils/create_distribution_plot.py |
Creates faceted boxplots with dynamic category ordering and color mapping |
tfbpshiny/modules/analysis_workspace.py |
Implements composite view rendering with filtering logic and empty state handling |
tfbpshiny/modules/analysis_sidebar.py |
Adds composite sidebar controls with method selection, dataset checkboxes, and threshold inputs |
tfbpshiny/mock_data.py |
Generates mock composite data with realistic statistical distributions and dataset mappings |
tfbpshiny/app.py |
Initializes composite-specific configuration defaults |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| passing_tfs = df.groupby("regulator_symbol")["passes"].any().loc[lambda s: s].index | ||
|
|
||
| # Mask DTO values that don't pass the threshold (set to NA so they don't |
There was a problem hiding this comment.
The comment refers to "Mask DTO values" but should be "Mask method values" or "Mask {method} values" since the code masks values for whichever method is selected (dto, rank_response_pvalue, or univariate_pvalue), not just DTO.
| # Mask DTO values that don't pass the threshold (set to NA so they don't | |
| # Mask selected method values that don't pass the threshold (set to NA so they don't |
| and the *y_column*. Values should already be display-ready strings. | ||
| :param y_column: Numeric column to plot on the y-axis. | ||
| :param y_axis_title: Display label for the y-axis. | ||
| :param kwargs: Forwarded to :func:`plot_formatter`. |
There was a problem hiding this comment.
The docstring mentions "kwargs: Forwarded to :func:plot_formatter" but the function signature uses *, height: int = 500 which is a single named parameter, not **kwargs. The docstring should be updated to mention only the height parameter explicitly, or changed to ":param height: Forwarded to :func:plot_formatter." for accuracy.
| :param kwargs: Forwarded to :func:`plot_formatter`. | |
| :param height: Forwarded to :func:`plot_formatter`. |
|
|
||
| # Mask DTO values that don't pass the threshold (set to NA so they don't | ||
| # appear in plot) | ||
| df.loc[~df["passes"], method] = pd.NA |
There was a problem hiding this comment.
Assigning pd.NA to a float64 column may trigger automatic dtype conversion to Float64 (nullable float) or object dtype, which could have unexpected performance or behavior implications. For better explicitness and to avoid potential issues with Plotly's handling of different dtypes, consider using np.nan instead, which maintains float64 dtype: df.loc[~df["passes"], method] = np.nan. This would require importing numpy as np.
|
|
||
| from .plot_formatter import plot_formatter | ||
|
|
||
| logger = logging.getLogger("shiny") |
There was a problem hiding this comment.
The logger is imported but never used in this module. Consider removing the unused import or adding appropriate logging statements for error conditions (e.g., when ValueError or TypeError are raised).
| "page": 1, | ||
| "page_size": 25, | ||
| "composite_method": "dto", | ||
| "composite_filter_threshold": 0.01, |
There was a problem hiding this comment.
The default value for composite_filter_threshold is 0.01 here, but the sidebar UI input widget in analysis_sidebar.py line 189 has value=0.5. This inconsistency means the UI will not reflect the actual default from the config when the composite module first loads. Consider making these values consistent, likely by changing the sidebar default to 0.01 to match the config default.
| "composite_filter_threshold": 0.01, | |
| "composite_filter_threshold": 0.5, |
|
Looks good, mack |
Migrated composite analysis from legacy code. Implemented dynamic boxplot. Added threshold filtering.