Repository navigation
Use peak calling for ChIP-exo and ChEC-seq in Figures tab - #300
Merged
michael-brent merged 9 commits intoOct 12, 2026
Merged
michael-brent merged 9 commits into
michael-brent merged 9 commits into
Conversation
Add FIGURE_BINDING_500BP (Calling Cards promoter enrichment, rossi_peaks_500bp, chec_m2025_peaks_500bp) and use it for figures 1, 2, 4, 5, 6 (defaults only) and 10. Figures 7/8 resolve peak-calling variants per promoter set for Rossi and ChEC-seq. Figures 3 and 9 are unchanged. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This PR is against the
|
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The snapshot oracle no longer covers two Figure 7 assays, and two changed lines fail pre-commit formatting checks.
4 open findings
What changed in this PR
Updates Figures to use 500bp peak-calling datasets for ChIP-exo and ChEC-seq while retaining promoter enrichment for Calling Cards.
Changes:
- Adds shared figure dataset defaults and applies them across Figures 1–8 and 10.
- Resolves peak-calling variants per promoter set for Figures 7–8.
- Updates materialization, documentation, and tests.
| File | Description |
|---|---|
tfbpshiny/modules/figures/ui.py |
Updates figure descriptions. |
tfbpshiny/modules/figures/server/fig7_9.py |
Resolves Figure 7–8 method-specific variants. |
tfbpshiny/modules/figures/server/fig10.py |
Updates Figure 10 documentation. |
tfbpshiny/modules/figures/queries.py |
Applies new figure dataset defaults. |
tfbpshiny/materialize/comparison/target_sets.py |
Materializes peak-based Figure 10 targets. |
tfbpshiny/datasets.py |
Defines FIGURE_BINDING_500BP. |
tests/unit/test_figures_queries.py |
Updates Figure 6 default assertions. |
tests/unit/test_figures_plots.py |
Adds peak-variant labels. |
docs/materialized_db_schema.md |
Documents revised datasets. |
CHANGELOG.md |
Records behavior and rebuild requirement. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| promoter_set, method = reg[db] | ||
| assert promoter_set == "500bp", f"{db} uses promoter set {promoter_set!r}" | ||
| assert method == "promoter_enrichment", f"{db} uses method {method!r}" | ||
| expected = "promoter_enrichment" if db == "callingcards_500bp" else "peak_calling" |
Comment on lines
+205
to
+209
| #: The binding datasets the Figures tab draws (figures 1, 2, 4, 5, 6 and 10): Calling | ||
| #: Cards promoter enrichment, plus ChIP-exo and ChEC-seq peak calling, all over the | ||
| #: 500bp start-codon window. Calling Cards has no peak-calling arm. Figures 3 and 9 | ||
| #: choose their own datasets, and figures 7 and 8 resolve variants per promoter set | ||
| #: from :data:`PROMOTER_ENRICHMENT_500BP`'s primaries. |
| #: Binding datasets shown in the figures, in display order. These are the *primary* | ||
| #: db_names -- the promoter-set and peak variants are not separate figure series. | ||
| BINDING_ORDER: tuple[str, ...] = ("harbison", *PROMOTER_ENRICHMENT_500BP) | ||
| BINDING_ORDER: tuple[str, ...] = ("harbison", *FIGURE_BINDING_500BP) |
Comment on lines
389
to
390
| " enrichment and ChIP-exo and ChEC-seq peak calling. Samples follow the dataset filters, which by" | ||
| " default leave one sample per TF in each dataset." |
cmatKhan
reviewed
Oct 9, 2026
cmatKhan
left a comment
cmatKhan
left a comment
Member
There was a problem hiding this comment.
the comments and documentation correctly describe the intended change. However, I see repeated code. Can that be reduced based on DRY principle? Additionally, the new global variable is not named in a way that makes its purpose obvious
| variant_cell[variant] = (b_primary, box_key) | ||
| return variant_cell | ||
|
|
||
| def _fig78_variant_cells() -> dict[str, tuple[str, str]]: |
Member
There was a problem hiding this comment.
this function is repeated in fig10.py and target_sets.py. Is there a good reason to have repeated code?
| b for b in PROMOTER_ENRICHMENT_500BP if b not in METHOD_COMPARISON_ASSAYS | ||
| ) | ||
| return { | ||
| **_resolve_variant_cells( |
Member
There was a problem hiding this comment.
explain what _resolve_variant_cells does
| #: 500bp start-codon window. Calling Cards has no peak-calling arm. Figures 3 and 9 | ||
| #: choose their own datasets, and figures 7 and 8 resolve variants per promoter set | ||
| #: from :data:`PROMOTER_ENRICHMENT_500BP`'s primaries. | ||
| FIGURE_BINDING_500BP: tuple[str, ...] = ( |
Member
There was a problem hiding this comment.
FIGURE_BINDING_500BP isn't obviously related to what this variable is used for. please make a more specific name
…ments, line lengths Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ture/299-peak-calling-in-figures # Conflicts: # CHANGELOG.md
…res' into feature/299-peak-calling-in-figures # Conflicts: # CHANGELOG.md
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #299
ChIP-exo and ChEC-seq now use peak calling instead of promoter enrichment on the Figures tab, still over the 500bp promoter set. Calling Cards has no peak-calling arm and stays promoter enrichment.
FIGURE_BINDING_500BP(callingcards_500bp,rossi_peaks_500bp,chec_m2025_peaks_500bp) drives figures 1, 2, 4, 5 and 10, and the figure 6 default selection (default settings only).topn_target_sets, so its binding panels need a database rebuild (noted in CHANGELOG).Not verified against a freshly built database; the local
brentlab_yeast.duckdbpredates the current schema.🤖 Generated with Claude Code