drighelli / SpatialExperiment

55 stars 20 forks source link

invalid .$sample_id-replacement currently passes with a warning #42

Closed HelenaLC closed 3 years ago

HelenaLC commented 3 years ago

Issue

In previous versions, the colData-replacement method assured that replacing the sample_id column in colData only passed provided that

  1. old and new IDs have a one-to-one mapping
  2. the number of unique IDs is retained
  3. IDs aren't set to NULL

Currently, breaking any of these gives a warning(), but the replacement is carried out regardless. As a result, entries in imgData are no-longer mappable to the cells, since sample IDs can be removed, arbitrarily reordered, or overwritten with a different number unique IDs.

Proposal

Revert to the previous replacement implementation, which

  1. stop()s invalid replacement
  2. passes replacement with NULL but retains sample_ids
HelenaLC commented 3 years ago

Current plan from meeting:

(assigned @lmweber to cover documentation, including some examples of invalid/valid replacements, and demonstrating how to cbind e.g. 2 experiments each with section1/2 as sample IDs; the rest is still open)

HelenaLC commented 3 years ago

fixed via #50