Add plot(GCIMSDataset) with a shared, comparable color scale - #55
Merged
Conversation
Adds a new plot() method for GCIMSDataset that renders several samples side by side (via cowplot::plot_grid()), all on the same intensity color scale, so they're visually comparable. The shared scale is controlled by a new intensity_range argument: "global" (default), "ranged", a fixed c(min, max), or a list/vector whose min/max are independently a number, "global" or "ranged". The resolver (resolve_intensity_range()) only evaluates whichever of "global"/"ranged" is actually referenced, and at most once. "global" is backed by a per-sample intensity range cached on the dataset (ds$IntensityRange, internal only), computed for free: it rides along inside .extract_RIC_and_TIS_fun_extract, which already loads each sample's full intensity matrix to compute RIC/TIS on every realize() -- adding range(intmat) costs one extra range() call on data already in memory, no new pass, no new DelayedOperation. Using the sample's full uncropped range (rather than scoping to dt_range/ rt_range/sample selection) is deliberate: it's always a safe bound and keeps the scale stable across differently-cropped or -subset calls. It doesn't apply with remove_baseline = TRUE (the cache holds raw intensities), which errors clearly toward intensity_range = "ranged" instead. plot(GCIMSSample) gained a fill_range parameter (default NULL, current auto-scaling behavior unchanged) to make one panel's color scale overridable, threaded through to mat_to_gplot() and mat_to_nativeRaster() (which already had an unused rangex override hook). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019V3CHRGSzcEu3k6tpUFv56
plot(GCIMSDataset) already resolved intensity_range via "global"/ "ranged"/fixed/list and passed the result straight into plot(GCIMSSample, fill_range = ...) -- same knob, two names depending which layer you were at. Renamed plot(GCIMSSample)'s parameter to intensity_range and gave it the same resolve_intensity_range() vocabulary (moved to utils-plot.R now that both methods share it). This isn't just a rename: "global" vs "ranged" is a real distinction even for one sample once dt_range/rt_range crop the view. "ranged" (the new default, matching prior behavior exactly) auto-scales to just what's shown; "global" scales against the sample's own full range instead, e.g. to see how strong a cropped region is relative to the whole sample. GCIMSSample has no cache (deliberately, given the staleness risk discussed earlier for per-sample derived data), so "global" is just range(intensity(object)) computed on demand -- cheap enough for a user-invoked plot call. Errors clearly, same as at the dataset level, if combined with remove_baseline = TRUE. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019V3CHRGSzcEu3k6tpUFv56
Rename the internal cache field IntensityRange -> intensity_range, to match the intensity_range argument it backs. intensity_range = "global" combined with remove_baseline = TRUE previously errored, since the dataset's cache (and the trivial range(intensity(x)) shortcut on GCIMSSample) both hold raw intensity, not baseline-removed. Instead of refusing the combination, compute it: each selected sample's full, uncropped intensity minus its full baseline, ranged across all of them. Same "global ignores dt_range/ rt_range" semantics as the raw case, just costing an extra pass instead of being free. Memoized so a list(min=, max=) spec resolving "global" for both endpoints only computes it once. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019V3CHRGSzcEu3k6tpUFv56
Replaces the unbounded "load and render every selected sample at once" default with paginated rendering, per discussion: - nrow/ncol define page capacity (nrow * ncol), replacing the earlier standalone ncol-only parameter -- matches ggforce::facet_wrap_paginate()'s convention rather than inventing a separate samples_per_page. When neither is given, both are picked from the sample count: an exact fit for 1-6 samples, otherwise capped at 3x3 (resolve_page_grid(), new in utils-plot.R). When exactly one is given, the other defaults to 3 if there are more than 9 samples, or just enough to fit everyone otherwise. - page (default 1) selects which page to render; out-of-bounds pages error clearly instead of silently clamping or rendering nothing. - sample= filters first (as before), pagination then slices that filtered set into pages -- no special-casing needed, just two sequential steps. - Rendering now loads only the current page's samples, one at a time, discarding each raw sample right after building its (much smaller, native-raster-encoded) panel -- so peak memory during rendering is bounded by page size, not dataset size. - intensity_range = "ranged" deliberately stays scoped to every selected sample across all pages, not just the current one: a page-local range would make pages incomparable to each other, defeating the point of paginating through a dataset to compare samples. This does mean it still costs a full pass over every selected sample's data (loaded and discarded one at a time, same memory bound as rendering) -- accepted as necessary for fair cross-page comparison. intensity_range = "global" is unaffected: still the cached, whole-dataset, page-independent range. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019V3CHRGSzcEu3k6tpUFv56
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.
Summary
plot(GCIMSDataset, sample =, dt_range =, rt_range =, ..., remove_baseline =, trans =, intensity_range = "global", ncol =)renders several samples side by side (viacowplot::plot_grid()), all on the same intensity color scale, so they're actually visually comparable — previously each sample plotted independently and auto-scaled to its own range.intensity_rangeaccepts"global"(default),"ranged", a fixedc(min, max), or a list/vector with independently-resolvedmin/max(number,"global", or"ranged"), resolved byresolve_intensity_range()(R/utils-plot.R), which only evaluates whichever source is actually referenced, at most once each."global"is backed by a per-sample raw intensity range cached on the dataset (ds$intensity_range, internal only — no S4 accessor). It's computed for free: it rides along inside.extract_RIC_and_TIS_fun_extract, which already loads each sample's full intensity matrix to compute RIC/TIS on everyrealize()— no new pass, no newDelayedOperation.plot(GCIMSSample)'s equivalent parameter (originally namedfill_range, added first as a simpler numeric-only override) was renamed tointensity_rangeand given the same"global"/"ranged"/fixed/list vocabulary, so both methods share one consistent parameter instead ofplot(GCIMSDataset)silently translating between two names internally."global"is a genuinely useful addition at the single-sample level too: it lets you see how strong a cropped region (dt_range/rt_range) is relative to the sample's own full range, instead of always auto-scaling to just what's shown.intensity_range = "global"combined withremove_baseline = TRUEis computed via an extra pass (each selected sample's full, uncropped intensity minus its full baseline) rather than being disallowed, since the raw-intensity cache/shortcut doesn't apply there.Test plan
testthatsuite passes, including new tests for the resolver's laziness/correctness (each source is only evaluated if actually referenced, and at most once), theintensity_rangecache (populated alongside TIS/RIC, reset on subset),plot(GCIMSSample)'s new"global"/"ranged"behavior, andplot(GCIMSDataset)itself (sample=selection, fixed limits, theremove_baseline+"global"two-pass path)align-GCIMSDataset.R/align-GCIMSSample.Rtests pass unchanged —getRIC()/getTIS()'s matrix-returning contract is untouchedGenerated by Claude Code