Skip to content

gsource: analyzeData + network refactor + accessor cleanup#361

Closed
jiajic wants to merge 81 commits into
giotto-suite:devfrom
jiajic:gsource
Closed

gsource: analyzeData + network refactor + accessor cleanup#361
jiajic wants to merge 81 commits into
giotto-suite:devfrom
jiajic:gsource

Conversation

@jiajic

@jiajic jiajic commented May 24, 2026

Copy link
Copy Markdown
Collaborator

Brings the long-running `gsource` branch up to `dev`. Contains several large refactor arcs plus today's accessor consolidation.

Major arcs

analyzeData / processData dispatch

  • HVG (covLoess / covGroups), QC (cell/feat stats), normalize (library/log), filter, and PCA (random/exact) verbs unified under `analyzeData` / `processData` / `filterData` / `reduceData` generics.
  • Param-object families (`covLoessParam`, `randomPcaParam`, etc.) decouple verb from backend; GiottoDisk's parquetExprStore provides streaming-backend methods.

Network creation consolidation

  • Unified `createNetwork()` for spatial and nearest-neighbor flows; old `createSpatialNetwork` / `createNearestNetwork` become thin wrappers.
  • `spatialNetworkObj` and `nnNetObj` migrated to igraph storage; slot renames (`@network`, `@type`). On-load migration handles legacy subobjects.
  • Auto-write to parquetEdgeStore via `gobject@source` when setting on a backed gobject.

Deprecations / cleanup

  • `@h5_file` slot deprecated; `createGiottoObject` path now uses `backend` arg consistently.
  • `readExprData` `expression_matrix_class` default is NULL (no coercion).
  • `overlapInfo` exported as an extension point.

Accessor refactor (today)

  • `.prune_nesting` applied uniformly across 6 setter NULL-removal sites — eliminates ~60 LOC of duplicated recursive-prune scaffolding.
  • New `.vmsg_null_remove(verbose, what)` helper standardizes the "NULL passed to x" message across 7 setters.
  • `wrap_msg` + `isTRUE(verbose)` blocks migrated to `vmsg(.v = verbose, ...)`.
  • Setter `verbose` defaults aligned to NULL (matches `setExpression` / `setPolygonInfo` / `setFeatureInfo` / `setGiottoImage`).
  • Net: ~190 LOC removed, zero behavior change.

Test plan

  • `devtools::test()` — 990 tests pass, 0 failures (verified against the GiottoDisk dev branch which carries the parquetEdgeStore + sourceWrite(igraph) work).
  • R CMD check across CI matrix
  • BiocCheck

Depends on GiottoDisk `dev` (parquetEdgeStore + Input/storeWrite work, already merged upstream).

- writes pts, poly, matrix data to disk
- save and load hooks for giottodisk
- deprecated param no longer does anything

This function now just for simple read-ins to dgCMatrix. More complex matrix formats usually need more handling that already wasn't addressed here. Coercion to DelayedArray also doesn't help much when the main goal for doing that would be to have a disk-backed representation
- also cleanup some deprecated code
jiajic and others added 28 commits May 19, 2026 20:03
Adds a third verb to the process / analyze taxonomy:

  - processData : transforms data (same-shape output)
  - analyzeData : computes summary stats (data.table output)
  - filterData  : computes selection / membership (ID list output)

filterParam is a new VIRTUAL processParam-sibling. Concrete subclasses
(e.g. defaultFilterParam in Giotto) extend it; backend-specific methods
on filterData dispatch on the param class (Giotto-side default method
for allMatrix / IterableMatrix; GiottoDisk-side streaming method for
parquetExprStore).

Also exports $/.DollarNames/$<- methods on filterParam, mirroring the
analyzeParam pattern, so param$detection_threshold style access works.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a fourth verb to the dispatch taxonomy for dimensionality reduction
/ decomposition operations:

  - processData : transforms data (same-shape output)
  - analyzeData : computes summary stats (data.table output)
  - filterData  : computes selection / membership (ID list output)
  - reduceData  : computes decomposition / embedding (list of matrices)

reduceParam is the VIRTUAL parent. Concrete reductions (Giotto's
pcaParam family, future UMAP/tSNE param classes) extend it. Backend-
specific methods on reduceData dispatch on the param class (Giotto-side
default methods for allMatrix; GiottoDisk-side streaming Halko on
parquetExprStore).

Also exports $/.DollarNames/$<- methods on reduceParam, mirroring the
analyzeParam/filterParam pattern.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds "parquetExprStore" (GiottoDisk's streaming expression backend) to
the accepted_classes vector in .evaluate_expr_matrix so createExprObj
accepts a parquetExprStore as expression_data and passes it through
unchanged. inherits() is harmless when GiottoDisk isn't installed --
no Imports/Suggests dependency added.

Fixes the "expression input needs to be ... matrix-like or data.frame-
like class" error that fired after Giotto's normalizeGiotto refactor
routed library + log norm through processData(parquetExprStore,
libraryNormParam/logNormParam) -- the dispatch returns a
parquetExprStore which createExprObj then rejected.

End-to-end verified: normalizeGiotto(g_parquet) now produces a
"normalized" exprObj whose exprMat is the same parquetExprStore with
JIT recipe (scale_factors, log, base) attached on @params$norm; no
parquet rewrite.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace apply(combn(...), MARGIN = 1L, ...) with direct column-indexed
flattening, and drop the no-op igraph round-trip (graph_from_edgelist ->
as_adjacency_matrix -> graph.adjacency -> get.data.frame) that used
deprecated igraph functions and never actually performed the undirected
dedup it appeared intended to do.

The new path canonicalizes (from, to) pairs via from < to and uses
data.table::unique() for dedup. Result: geometry backend now emits the
same undirected edge set as deldir and RTriangle (verified equal on the
spatLocsObj test fixture: 1275 edges, identical pairs across all three
backends).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Convert createNetwork from a standalone function to an S4 generic
dispatching on (data class, networkParam) signature, matching the
processData / filterData / reduceData / analyzeData family pattern but
slotting into the create<Noun> object-construction family.

Class hierarchy:
  networkParam (virtual, in R/classes-utils.R)
  +- NNNetworkParam (virtual)
  |  +- kNNNetworkParam    (slots: k, filter, maximum_distance, ...)
  |  +- sNNNetworkParam    (slots: k, top_shared, minimum_shared, ...)
  +- delaunayNetworkParam  (slots: method, options, Y, j, S, ...)

Each Param exposes an `output` slot taking "auto" | "data.table" |
"igraph" | "parquet", replacing the old `as.igraph` boolean. "auto"
resolves based on supplied data class and `backend` arg per the
GiottoDisk-bridge design.

Behaviour changes (intentional, baked into the consolidation):
  - sNN is now undirected at creation (one edge per symmetric pair) when
    promoted to igraph. Clustering already symmetrises downstream via
    igraph::as.undirected(), so observable impact is nil.
  - Delaunay (all three backends) is now undirected at creation. Matches
    the geometric reality and was the implicit intent of the broken
    igraph round-trip removed in the prior perf commit.
  - sNN `rank` column is dropped from the output. It was per-source and
    ill-defined post-symmetrisation; not consumed by any downstream code.
    `shared` (a true symmetric edge attribute) is kept.

The legacy createNetwork(x, type = ..., method = ..., as.igraph = ...,
...) signature is preserved via setMethod(signature("matrix", "missing"))
that translates string args into the appropriate Param and dispatches.
All 64 tests in test_10_create_network.R pass unchanged through this
shim. Updated one assertion in test-networks.R to compare sNN paths on
their canonical undirected edge set (the legacy createNearestNetwork
still emits the duplicated-arc directed form until it becomes a thin
wrapper in a follow-up commit).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…otto

Add S4 methods so that createNetwork accepts the common Giotto data
containers directly, each delegating to the matrix method after pulling
the appropriate underlying matrix:

  - signature("spatLocsObj", "networkParam"): extracts sdim[xyz] columns
    + cell_ID for node_ids.
  - signature("dimObj", "networkParam"): extracts the reduction matrix;
    optional `dimensions_to_use` slices PC columns.
  - signature("giotto", "NNNetworkParam"): pulls the configured dimObj
    (default "pca", feat-type aware), then delegates to dimObj method.
  - signature("giotto", "delaunayNetworkParam"): pulls the spatLocsObj
    (default spat_loc_name = "raw"), then delegates to spatLocsObj method.

The giotto method returns the bare network (data.table/igraph/parquet
per the Param's output slot) — gobject-storage wrapping stays in the
createNearestNetwork / createSpatial*Network thin wrappers that come
next. This mirrors the clean separation in clusterData where the
gobject method returns labels and the wrappers handle storage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a `space` parameter to the signature("giotto", "NNNetworkParam")
method with choices c("expression", "spatial"). Default "expression"
preserves existing behaviour (pulls a dimension reduction such as PCA).
Setting space = "spatial" routes through the spatLocsObj dispatch —
enabling one-call spatial kNN/sNN construction without going through
the upcoming createSpatialKNNnetwork wrapper.

The arg is only exposed on the NN method. The delaunayNetworkParam
method is unchanged — its source is always spatial locations and
adding a no-op `space` arg there would falsely suggest users need
to think about it.

Also switch both giotto methods to the exported accessors
getSpatialLocations() and getDimReduction() (instead of the internal
get_spatial_locations / get_dimReduction used in the prior commit).
Matches the direction of the accessor_rework branch — all new code
should reach into the gobject through exported entry points.

  # expression-space kNN (default)
  createNetwork(g, kNNNetworkParam(k = 30))
  # spatial kNN
  createNetwork(g, kNNNetworkParam(k = 6), space = "spatial")
  # delaunay (no space arg)
  createNetwork(g, delaunayNetworkParam(method = "deldir"))

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…work

Add tests that pin load-bearing behaviour of the legacy network-creation
wrappers ahead of the upcoming rewrite onto createNetwork + Param. Each
test exercises a path that's at risk of silent drift during the
refactor:

  - 3D Delaunay only works with the delaunayn_geometry backend; deldir
    and RTriangle error on 3D input.
  - maximum_distance_delaunay drops edges longer than the cutoff;
    "auto" mode applies a finite filter (<= edge count of no filter).
  - minimum_k re-adds nearest neighbours under a distance filter, so
    the resulting edge count is >= the bare distance-filter case.
  - delaunay_method = "delaunayn_geometry" routes to the geometry
    backend and produces the same undirected edge set as deldir.
  - Default name conventions: Delaunay_network, kNN_network for
    spatial; kNN.pca, sNN.pca for nearest-neighbour.
  - createNearestNetwork propagates the source dimObj's provenance
    into the resulting nnNetObj.

Intentionally NOT locked: coord columns on spatialNetworkObj
(sdimx_begin etc.) — slated for removal in a follow-up so networks
carry only edges and consumers fetch coords on demand.

All 78 tests in this file + 20 in test-networks.R pass against the
current implementation.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace the ~230-line inline implementation of createNearestNetwork
with a thin wrapper that builds the appropriate Param and delegates to
the createNetwork generic. The wrapper now:

  1. Builds kNNNetworkParam / sNNNetworkParam (output = "igraph") from
     the legacy args (k, minimum_shared, top_shared).
  2. Branches on dim_reduction_to_use:
     - non-NULL: dispatches createNetwork(dim_obj, param, ...) via the
       dimObj method.
     - NULL: pulls expression matrix, dispatches createNetwork(matrix,
       param, ...) via the matrix method.
  3. Wraps the returned igraph in nnNetObj with provenance pulled from
     the source dimObj / exprObj.
  4. Stores via setNearestNetwork() + update_giotto_params() when
     return_gobject = TRUE.

All ~230 lines of inline dbscan plumbing, manual data.table assembly,
and cell-name handling are now centralized in createNetwork() — so
behavior is identical to the new generic (including the directionality
fixes: sNN now undirected at creation, one edge per symmetric pair).

Cross-impl parity restored: the sNN equivalence test in test-networks.R
that had been comparing on undirected edge sets (because the old legacy
emitted directed-with-duplicates form) now passes strict
igraph::identical_graphs() since both paths flow through the same
createNetwork core.

One error message updated: missing dim reduction now reports via
getDimReduction's standard "not found" message rather than the
legacy's "is not available" — message is more informative (includes
spat_unit, feat_type, name).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Major restructure of the two network subobject classes onto a unified
igraph-canonical storage form. Breaking changes are gated on
GiottoClass 0.6.0 via the existing updateGiottoObject() migration so
legacy serialized gobjects load cleanly under the new schema.

Slot renames (both happen at the virtual parent class level):
  nnData@igraph              -> nnData@network
  spatNetData@networkDT      -> spatNetData@network        (type now "ANY")
  spatNetData@networkDT_before_filter -> spatNetData@unfiltered

Storage shape change:
  spatialNetworkObj@network is now an igraph (was data.table). The
  prior data.table form is no longer carried — `getSpatialNetwork()`
  / `sn[]` returns the igraph directly, matching nnNetObj. Edges no
  longer carry sdimx_begin/y_begin/x_end/y_end coordinate columns;
  consumers that need geometry pair the network with the spatLocsObj
  on demand.

Geometric-transform methods removed (graph topology is invariant
under translation/rotation/flip; coordinates live in spatLocsObj):
  spatShift, flip, t, crop, ext on spatialNetworkObj. Helpers
  .shift_spatial_network and .flip_spatnet deleted.

Constructors updated:
  createSpatNetObj(network=, unfiltered=, ...) -- renamed args.
  create_nn_net_obj(network=, ...) -- renamed arg.
  .evaluate_spatial_network now accepts igraph or data.frame
  (from/to required) and dropped the coord-column requirement.

Migration in updateGiottoObject:
  .update_network_slots() walks g[["spatial_network"]] and
  g[["nn_network"]] (lists of subobjects via the existing
  flatten-by-bracket accessor), reads legacy data via attr() against
  old slot names, rebuilds each subobject via create_*_obj with the
  new schema. spatialNetworkObj legacy DTs are converted to undirected
  igraphs preserving from/to/distance/weight.

Spatial wrappers reimplemented as thin wrappers over createNetwork():
  createSpatialDelaunayNetwork, createSpatialKNNnetwork, and (by
  consequence) createSpatialNetwork now build the appropriate
  *NetworkParam, dispatch via the matrix method, wrap the result in a
  spatialNetworkObj, and store with update_giotto_params(toplevel=1L).
  Deprecated internal helpers removed: .create_delaunaynetwork_geometry
  / _geometry_3d / _RTriangle / _deldir / _2d / _3d, and
  create_KNNnetwork_dbscan -- all subsumed by createNetwork's matrix
  dispatch.

Tests updated for the new schema: igraph-shaped assertions
(igraph::ecount, igraph::E()$weight) replace the old DT-shaped ones.
All 78 tests in test_10_create_network.R pass on the migrated
fixture; test-networks.R updates pending follow-up consumer rewires.

Net: 38 files, +295 / -1583 lines.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…dioms

Two legacy helpers operating on the deprecated coord-carrying DT
schema are removed:
  - convert_to_full_spatial_network()
  - convert_to_reduced_spatial_network()

They existed as a workaround for igraph-flavoured semantics in a DT
representation. With spatialNetworkObj@network now an undirected
igraph, the workaround is unnecessary — both-directions semantics are
native, and consumers that genuinely need a full DT view can build
it with one rbind on canonical edges.

In-package rewires:
  - .filter_network (R/spatial_structures.R): replaced expand →
    filter → reduce cycle with two-pass per-side rank computation
    (rank_from / rank_to via setorder + by). Same minimum_k floor
    semantic, no rbind, single DT throughout.
  - annotateSpatialNetwork: drops the cached coord-column rename
    block; now joins sdim*_begin/end coords from the live
    spatLocsObj so its output contract is preserved without relying
    on stored coords. Both-directions expansion via direct rbind.
  - .clp_group_spatialnetwork (combine_metadata.R): direct rbind +
    rename instead of convert_to_full call.
  - .clp_group_annotation second site (combine_metadata.R): same.

getSpatialNetwork output options:
  - Drop the data.table::copy() branch — @network is igraph now
    (copy-on-modify; copy_obj is a no-op for the network contents).
  - Add "igraph" as an explicit output option.
  - "networkDT" still works but converts via igraph::as_data_frame
    on the way out (legacy-friendly path).
  - Rename "networkDT_before_filter" choice to "unfiltered" to match
    the slot name.
  - Internal note: accessor_rework branch will need to merge these
    output-handling changes; minor conflicts expected.

External consumers in GiottoVisuals and Giotto still call
convert_to_full_spatial_network and read sdim*_begin coord columns
from getSpatialNetwork output. Those rewires are tracked as
follow-ups; this PR is GiottoClass-internal.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wire up the remaining pieces so the full GiottoClass test suite passes
on the new schema.

Subset / crop network pruning:
  .subset_spatial_network now uses igraph::induced_subgraph to prune
  the network by the surviving cell set, replacing the legacy DT
  `to %in% cell_ids & from %in% cell_ids` filter. Matches the existing
  .subset_nearest_network pattern. This restores the gobject-level
  subset/crop pruning that was previously handled by spatialNetworkObj's
  own crop method (deleted alongside the other geometric transforms).

createSpatialWeightMatrix:
  Rewired to use igraph::as_adjacency_matrix directly on the canonical
  @network slot. Drops the DT round-trip + redundant graph_from_data_frame
  reconstruction.

Subobject-level legacy migration:
  Added initialize methods for spatNetData and nnData that detect
  pre-0.6.0 slot layouts (presence of attr(., "networkDT") or
  attr(., "igraph")) and route through .migrate_spatnet_obj /
  .migrate_nn_net_obj. This covers the path where a standalone legacy
  subobject (e.g. from loadSubObjectMini) is set into a fresh gobject
  via setSpatialNetwork / setNearestNetwork — the setters now call
  methods::initialize() on the input before dispatching, triggering
  migration there too.

data_input.R::.read_spatial_networks: rename a stale
  networkDT_before_filter argname → unfiltered (matches the new
  createSpatNetObj signature).

Tests:
  - test-networks.R: gutted the obsolete convert_to_full_spatial_network
    parity test and the cross-impl parity tests (now redundant since
    createSpatial*Network and createNetwork share an implementation).
    Replaced with focused wrapper smoke tests.
  - test-createObject.R / setup.R: initialize() the network subobject
    minis on load so their legacy schema is normalized.

Verified: 971 tests pass across the full GiottoClass suite.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
… path

Three latent bugs from the half-finished h5_file deprecation, all caught
running the test suite post dev→gsource merge.

1. createGiottoObject (R/create.R): the file-existence check on h5_file
   at line ~369 was missed when h5_file was deprecated to `backend`.
   h5_file is now `lifecycle::deprecated()` (a sentinel, not NULL), so
   `!is.null(h5_file)` is TRUE and `file.exists(deprecated_sentinel)`
   errors. Removed the dead block — path-handling already happens
   earlier via `is_present(h5_file)` and the backend = ... routing.

2. setPolygonInfo (R/slot_accessors.R): the call to
   setSpatialLocations(gobject, spatlocs, ...) sat outside the
   `if (centroids_to_spatlocs)` block defining `spatlocs`. With the
   default `centroids_to_spatlocs = FALSE`, `spatlocs` is undefined and
   the call errors. Moved inside the conditional.

3. gefToGiotto / anndataToGiotto (R/interoperability.R): both forwarded
   h5_file to createGiottoObject as a still-valid arg. Migrated both
   to the same `backend` + `h5_file = deprecated()` shim pattern that
   createGiottoObject uses, including doc updates.

Plus a small follow-on in R/methods-nesting.R: spatUnit<- and
featType<- on giotto now pass `source = x@source` (was
`h5_file = x@h5_file`) when rebuilding the gobject — keeps the gsource
backend reference attached through the spat_unit / feat_type rename
path. h5_file is still a class slot with NULL default for back-compat;
removing it can be a separate cleanup pass.

Verified against pre-merge gsource HEAD^1 — same failure footprints
reproduce there, confirming these are gsource-vintage regressions, not
introduced by the dev merge.

Tests: 0 failures / 967 passes on the merged tip after fixes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Cascade: dev → gsource → analyzedata → consolidate-network-creation.
Resolves conflicts in NN_network.R, auxilliary.R, slot_accessors.R,
spatial_structures.R, subset.R.

Resolution highlights:
- Take HEAD's thin-wrapper createNearestNetwork / createSpatial*Network
  bodies (analyzedata still carried the legacy monoliths).
- Take HEAD's igraph-aware getSpatialNetwork output choices
  (spatialNetworkObj/igraph/networkDT/unfiltered/outputObj) and drop the
  copy_obj data.table copy path (igraph is copy-on-modify).
- Take analyzedata's access_rework setMethod-based set/getNearestNetwork
  and setSpatialNetwork (drop dead internal snake_case accessors).
- Preserve the pre-0.6.0 → 0.6.0 migration hook by calling
  methods::initialize(x) inside the camelCase setters before slot assign,
  and on getNearestNetwork before returning.
- Fix slot rename leftover in analyzedata's getNearestNetwork
  (slot(nnNet, "igraph") → "network", get.data.frame → as_data_frame).
- Drop the spatial-network setalloccol block in update_giotto_object_alloc_col
  since networks no longer store data.tables.
- Inline gobject subset to use setSpatialNetwork (camelCase).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Post-merge cleanup. The cascade dragged in analyzedata's getNearestNetwork
which still referenced the legacy @igraph slot (renamed to @network in the
DT→igraph migration on this branch). Also switch igraph::get.data.frame()
to the non-deprecated as_data_frame(what = "edges").

No silent on-read migration: legacy nnNetObj should surface as an error so
the user runs updateGiottoObject() rather than rely on the getter to fix
schema mismatches.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
methods-extract.R: spatNetData / nnData `[` doc sections now consistently
read "@network slot (an igraph)".

interoperability.R: refer to getNearestNetwork (the camelCase S4 generic),
not the removed get_NearestNetwork internal. Fix "hanldling" typo.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds new / changes / breaking-changes coverage for the
spatialNetworkObj + nnNetObj → igraph migration, the createNetwork()
S4 generic + networkParam family, the createSpatial*Network /
createNearestNetwork thin-wrapper rewrite, and the geometric-transform
method removals on spatialNetworkObj.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feat: 0.6.0 cascade — network refactor + verb-split + access_rework
setNearestNetwork() and setSpatialNetwork() now mirror the pattern in
setExpression / setPolygonInfo / setFeatureInfo: when the gobject has
a gsource backend attached and the incoming network's @network slot
is in-memory (igraph, not a dataStore), write it through to a
parquetEdgeStore so the artifact lands in the project vault.

Caller can still control output explicitly via networkParam(..., output
= "parquet") at createNetwork time. This is the safety net for users
who pass in-memory igraphs — they get the disk-backed form for free
when the gobject already has a backend.

GiottoDisk-side dispatch added in a paired commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three new tests in test-networks.R covering the auto-write path that
landed in commit b57c52d:

- setNearestNetwork auto-writes igraph to parquetEdgeStore when the
  gobject has a gsource backend attached
- setSpatialNetwork does the same
- Both setters leave in-mem igraphs untouched when the gobject has no
  backend (no false promotion)

All three skip via skip_if_not_installed("GiottoDisk") so the test
file remains runnable without the optional disk dep.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Auto-write block in setNearestNetwork / setSpatialNetwork was passing
the in-mem igraph to sourceWrite without a type arg, so the resulting
parquetEdgeStore inherited storeWrite's default `type = "kNN"`
regardless of what kind of network it actually was. sNN networks
landed as @type = "kNN"; spatial networks ditto.

- setNearestNetwork now passes `type = x@nn_type` (kNN/sNN).
- setSpatialNetwork passes `type = "spatial"` (the network's
  construction method — delaunay/kNN/Voronoi — lives on the
  spatialNetworkObj's @method slot, not the parquetEdgeStore).

@directed continues to be auto-detected from igraph::is_directed()
inside storeWrite — that wiring was already correct.

Test added covering all three cases (sNN, kNN, spatial) — verifies
both @type and @directed land correctly on the resulting store.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
feat: auto writes of networks to backed projects
setSpatialLocations, setDimReduction,
setNearestNetwork, setSpatialNetwork, setPolygonInfo, and
setSpatialEnrichment each had the same recursive empty-list prune
inlined as a manually-nested if-cascade. .prune_nesting() already
existed for setExpression — extending it to the other 6 setters
eliminates ~60 lines of duplicate scaffolding.

Also adds .vmsg_null_remove(verbose, what) to standardize the
'NULL passed to x. Removing specified <noun>.' message that
appeared at each of those sites, with minor wording inconsistencies
(some used wrap_msg + isTRUE(verbose), some used vmsg directly).
All callsites now route through one helper.

Net: 34 insertions / 90 deletions. No behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Each setter had blocks like

    if (isTRUE(verbose)) {
        wrap_msg("...")
    }

which collapse to a single vmsg(.v = verbose, "...") call — same
behavior, less scaffolding. Touched setters: setCellMetadata,
setFeatMetadata, setMultiomics, setSpatialLocations, setDimReduction,
setNearestNetwork, setSpatialNetwork, setSpatialGrid, setSpatialEnrichment.

Also includes:
- One wrap_txtf + isTRUE(verbose) site in setPolygonInfo's
  polygon_name auto-pick block — converted to vmsg(... sprintf(...)).
- The NULL-removal blocks for setCellMetadata / setFeatMetadata /
  setMultiomics / setSpatialGrid now route through the
  .vmsg_null_remove helper added in the previous commit.

Bare wrap_msg calls in getters (getNearestNetwork, getSpatialEnrichment)
without a `verbose` param are intentionally left alone — they're
unconditional user-facing fallbacks.

No behavior change.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Now that every setter's verbose param is consumed only through
vmsg() / .vmsg_null_remove() (both honor verbose = NULL via the
package's giotto.verbose option resolution), align the defaults
with setExpression / setPolygonInfo / setFeatureInfo / setGiottoImage
which already use verbose = NULL.

Touched setters: setCellMetadata, setFeatureMetadata, setMultiomics,
setSpatialLocations, setDimReduction, setNearestNetwork,
setSpatialNetwork, setSpatialGrid, setSpatialEnrichment.

No behavior change for callers who pass verbose = TRUE/FALSE
explicitly; default callers now get whatever giotto.verbose resolves
to instead of unconditional TRUE.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jiajic jiajic closed this May 24, 2026

@github-advanced-security github-advanced-security AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lintr found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants