Skip to content

Simplify default specification and remove unused code#238

Open
Bisaloo wants to merge 3 commits into
mainfrom
simplify-20260606
Open

Simplify default specification and remove unused code#238
Bisaloo wants to merge 3 commits into
mainfrom
simplify-20260606

Conversation

@Bisaloo

@Bisaloo Bisaloo commented Jun 6, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

Comment thread R/sdAttrs.R
Comment thread R/sdAttrs.R
stopifnot(length(value) %in% c(1, ncol(x)))
stopifnot(all(nchar(value) > 0, na.rm=TRUE))
if (is.null(rk <- region_key(x))) region_key(x) <- "region"
rk <- region_key(x) <- region_key(x) %||% "region"

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

With the change, rk is also set to "region" if region_key(x) was missing. This was not the case before but it looks like maybe a bug.

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.

Agreed. setReplaceMethod("regions" does this too: default to "region" is missing.

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.

Alright, it passed (but took 1.5h) :)) We gotta keep remotes: for now, however, until spatialdataR is updated regularly upstream. I am avoiding frequent updates to not confuse the reviewer.

@Bisaloo Bisaloo force-pushed the simplify-20260606 branch from 6d29f28 to 2482505 Compare June 6, 2026 20:35
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