Skip to content

Sync shapes when dropping elements from table - #183

Closed
Bisaloo wants to merge 1 commit into
HelenaLC:mainfrom
Bisaloo:sync-shapes
Closed

Sync shapes when dropping elements from table#183
Bisaloo wants to merge 1 commit into
HelenaLC:mainfrom
Bisaloo:sync-shapes

Conversation

@Bisaloo

@Bisaloo Bisaloo commented May 8, 2026

Copy link
Copy Markdown
Collaborator

Currently, SpatialData includes utilities to drop elements from tables when removed from the other layers, but no utilities to drop elements from other layers when elements from tables are dropped.

This is a quick and dirty new .sync_shapes_on_drop() function, modelled directly after .sync_tables_on_drop() to address a use case needed in demos.

Eventually, the sync... functions should probably be refactored and all cases should be covered, but this should solve the specific issue at hand.

Comment thread R/utils.R
s <- shapes(x)[[i]]
# check which cells still exist
# FIXME: I kind of doubt this always has this name. Do we have an accessor for this???
keep <- s[["__index_level_0__"]] %in% all_nms

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Not ideal either, but I know it is sometimes i) missing, or ii) defined by instance_key of the table - maybe we can check for instance_key, and otherwise something like setdiff(names(s), c("radius", "geometry")? At least a little more robust, until we learn better...

@HelenaLC

HelenaLC commented May 8, 2026

Copy link
Copy Markdown
Owner

Let me talk to Daria/Luca before proceeding. I believe in this particular case, the table is linked to a shape. Said shape, however, misses an instance_key altogether. The column you see should be called "cell_id" imo. I just want to avoid us fixing things when the object is invalid in the first place (not sure yet, but might be)...

HelenaLC added a commit that referenced this pull request May 12, 2026
@HelenaLC

Copy link
Copy Markdown
Owner

Tried to address this with #194, but this assumes that instance_key exists in the linked shape ... Could close this but maybe put a new issue re: "missing instance key as column" discussed here?

@HelenaLC

HelenaLC commented Jun 1, 2026

Copy link
Copy Markdown
Owner

This has been addressed (see code/utils.R and methods.R), but there is certainly room for improvement in the future.

@HelenaLC HelenaLC closed this Jun 1, 2026
HelenaLC added a commit that referenced this pull request Jul 5, 2026
Former-commit-id: dfce8f8
Former-commit-id: dd815be4250550d75451a5a10b88bfd8a1962e9d
HelenaLC added a commit that referenced this pull request Jul 6, 2026
Former-commit-id: e6c5082395282c0df7a78a8800a8b49cdfa1fd64
Former-commit-id: dd815be4250550d75451a5a10b88bfd8a1962e9d
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