Polish real-world example gallery - #342
Conversation
📝 WalkthroughWalkthroughSix real-world visualization notebooks update chart rendering, axis visibility, themes, annotations, color palettes, data display limits, and layout padding. The Pan-UKBB notebook also adds capped Y rendering and off-scale peak labels. ChangesReal-world notebook visualization refresh
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Merging this PR will not alter performance
Comparing Footnotes
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
examples/real_world/03_pan_ukbb_manhattan.ipynb (1)
182-200: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winMismatched thresholds between peak clamp and OFF-SCALE label.
peak_display_yclamps tooff_scale_y(172) but the "OFF-SCALE" suffix only triggers when the raw value exceedsdisplay_y_max(180). A peak whose true value falls between 172 and 180 gets silently pinned to y=172 (visually displaced from its real position) with no annotation explaining the discrepancy, since 172 < value ≤ 180 doesn't satisfyvalue > display_y_max.🐛 Proposed fix: align clamp/suffix thresholds
- value = float(neglog10_pvalue[peak_index])\n", - suffix = " · OFF-SCALE" if value > display_y_max else ""\n", + value = float(neglog10_pvalue[peak_index])\n", + suffix = " · OFF-SCALE" if value > off_scale_y else ""\n", peak_labels.append(\n", f"CHR {chromosome} · {position / 1e6:.1f} MB · −LOG₁₀(P) {value:.1f}{suffix}" # noqa: RUF001\n", )\n", -peak_display_y = np.minimum(\n", - neglog10_pvalue[[chr20_peak_index, other_peak_index]],\n", - off_scale_y,\n", -)\n", +raw_peak_y = neglog10_pvalue[[chr20_peak_index, other_peak_index]]\n", +peak_display_y = np.where(raw_peak_y > off_scale_y, off_scale_y, raw_peak_y)\n",🤖 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 `@examples/real_world/03_pan_ukbb_manhattan.ipynb` around lines 182 - 200, Align the OFF-SCALE suffix condition in the peak_labels loop with the peak_display_y clamp by using off_scale_y as the threshold instead of display_y_max. Preserve the existing label formatting and ensure every peak whose value exceeds the clamp threshold is marked as OFF-SCALE.
🤖 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.
Outside diff comments:
In `@examples/real_world/03_pan_ukbb_manhattan.ipynb`:
- Around line 182-200: Align the OFF-SCALE suffix condition in the peak_labels
loop with the peak_display_y clamp by using off_scale_y as the threshold instead
of display_y_max. Preserve the existing label formatting and ensure every peak
whose value exceeds the clamp threshold is marked as OFF-SCALE.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 681f063d-4bc2-41a2-aa78-d9d8d654c8f2
⛔ Files ignored due to path filters (6)
examples/real_world/assets/01-gaia-hr-diagram.pngis excluded by!**/*.pngexamples/real_world/assets/02-gnomad-allele-frequency.pngis excluded by!**/*.pngexamples/real_world/assets/03-pan-ukbb-manhattan.pngis excluded by!**/*.pngexamples/real_world/assets/04-dukascopy-fx-ticks.pngis excluded by!**/*.pngexamples/real_world/assets/05-ligo-gw150914-strain.pngis excluded by!**/*.pngexamples/real_world/assets/06-nyc-taxi-density.pngis excluded by!**/*.png
📒 Files selected for processing (6)
examples/real_world/01_gaia_hr_diagram.ipynbexamples/real_world/02_gnomad_allele_frequency.ipynbexamples/real_world/03_pan_ukbb_manhattan.ipynbexamples/real_world/04_dukascopy_fx_ticks.ipynbexamples/real_world/05_ligo_gw150914_strain.ipynbexamples/real_world/06_nyc_taxi_density.ipynb
Summary
Validation
Closes ENG-10638.
Summary by CodeRabbit