New cf_add_meta function by jefferis · Pull Request #55 · natverse/coconatfly

jefferis · 2026-04-01T07:12:07Z

Add metadata columns based on one or more key columns e.g. pre/post metadata

New exported function for enriching dataframes containing neuron keys with metadata. Supports: - Single or multiple key columns - Configurable suffixes for multiple columns - Optional column selection via cols parameter - Warning for missing column names Also updates to exclude tissue/sex columns from metadata join since these are typically already present.

Changes details from boolean to character options: - "partner" (default): Add metadata for partner neurons only - "query": Add metadata for query neurons only - "both": Add metadata for both sides with .pre/.post suffixes - "neither": No metadata, return minimal columns for speed The implementation: - Keeps only core connectivity columns (pre_id, post_id, weight) - Uses cf_add_meta() to add metadata consistently from cf_meta() - Removes the add_partner_metadata() helper function Includes comprehensive tests for all details options and cf_add_meta functionality.

* nb this replaces a bad commit from claude

Copilot

Pull request overview

Adds a new cf_add_meta() helper and extends cf_partners() with a details option to control which side(s) of a partner table get metadata enrichment, enabling faster “minimal” partner queries and deferred metadata joins.

Changes:

Added details = c("partner","query","both","neither") to cf_partners() and refactored enrichment to use the new cf_add_meta().
Introduced cf_add_meta() (exported) for joining cf_meta()-derived columns onto one or more key columns with optional suffixing and column selection.
Added test coverage for details behavior and for cf_add_meta() joining options.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`R/partners.R`	Adds `details` arg and switches metadata enrichment to `cf_add_meta()` with key-based joins.
`R/meta.R`	Implements new exported `cf_add_meta()` helper.
`tests/testthat/test-partners.R`	Adds tests for `cf_partners(details=...)` and `cf_add_meta()` behavior.
`NAMESPACE`	Exports `cf_add_meta`.
`man/cf_partners.Rd`	Documents new `details` parameter.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-01T07:17:53Z

+    # Enrich with metadata based on details option
+    # Always use cf_add_meta rather than relying on partnerfun metadata
+    if (details != "neither" && nrow(tres) > 0) {
+      # Keep only core connectivity columns, drop all partnerfun metadata
+      # cf_add_meta will re-add metadata from cf_meta for consistency
+      core_cols <- c("pre_id", "post_id", "weight", "dataset", "tissue", "sex",
+                     "pre_key", "post_key")
+      tres <- tres[, intersect(names(tres), core_cols), drop = FALSE]
+


When details='neither', this code path only skips cf_add_meta but does not remove any metadata that partnerfun may already have returned. Several built-in partnerfuns (e.g. fanc/banc) left_join metadata before returning, so details='neither' would still include type/class/etc and violate the documented contract. Consider always stripping metadata columns (or subsetting to core connectivity columns) when details=='neither'.

Suggested change

# Enrich with metadata based on details option

# Always use cf_add_meta rather than relying on partnerfun metadata

if (details != "neither" && nrow(tres) > 0) {

# Keep only core connectivity columns, drop all partnerfun metadata

# cf_add_meta will re-add metadata from cf_meta for consistency

core_cols <- c("pre_id", "post_id", "weight", "dataset", "tissue", "sex",

"pre_key", "post_key")

tres <- tres[, intersect(names(tres), core_cols), drop = FALSE]

# Always drop partnerfun-specific metadata, keep only core connectivity columns

# cf_add_meta will re-add standardized metadata from cf_meta as needed

core_cols <- c("pre_id", "post_id", "weight", "dataset", "tissue", "sex",

"pre_key", "post_key")

tres <- tres[, intersect(names(tres), core_cols), drop = FALSE]

# Enrich with metadata based on details option

# Always use cf_add_meta rather than relying on partnerfun metadata

if (details != "neither" && nrow(tres) > 0) {

Copilot · 2026-04-01T07:17:54Z

+    # Always use cf_add_meta rather than relying on partnerfun metadata
+    if (details != "neither" && nrow(tres) > 0) {
+      # Keep only core connectivity columns, drop all partnerfun metadata
+      # cf_add_meta will re-add metadata from cf_meta for consistency
+      core_cols <- c("pre_id", "post_id", "weight", "dataset", "tissue", "sex",
+                     "pre_key", "post_key")
+      tres <- tres[, intersect(names(tres), core_cols), drop = FALSE]
+


Subsetting to core_cols drops all partnerfun-provided columns beyond the core connectivity fields (including potentially non-metadata columns). This also makes keep.all ineffective for preserving dataset-specific extra columns across datasets. If the goal is to ignore partnerfun metadata only, consider dropping just the known metadata columns (or only those that overlap cf_meta outputs) while preserving any additional connectivity metrics.

Suggested change

# Always use cf_add_meta rather than relying on partnerfun metadata

if (details != "neither" && nrow(tres) > 0) {

# Keep only core connectivity columns, drop all partnerfun metadata

# cf_add_meta will re-add metadata from cf_meta for consistency

core_cols <- c("pre_id", "post_id", "weight", "dataset", "tissue", "sex",

"pre_key", "post_key")

tres <- tres[, intersect(names(tres), core_cols), drop = FALSE]

# Always use cf_add_meta for standardized metadata; preserve any

# additional partnerfun-provided columns (e.g. extra connectivity metrics)

if (details != "neither" && nrow(tres) > 0) {

Copilot · 2026-04-01T07:17:54Z

+#' @examples
+#' \dontrun{
+#' # Add metadata to partner column only
+#' partners <- cf_partners(cf_ids(hemibrain='DA2_lPN'), threshold=10, details=FALSE)


The example uses details=FALSE, but cf_partners() now defines details as a character option (partner/query/both/neither). As written, this example will error; update it to a valid value (e.g. details='neither') or remove the argument if the default is intended.

Suggested change

#' partners <- cf_partners(cf_ids(hemibrain='DA2_lPN'), threshold=10, details=FALSE)

#' partners <- cf_partners(cf_ids(hemibrain='DA2_lPN'), threshold=10, details='neither')

Copilot · 2026-04-01T07:17:54Z

+
+  # Extract unique keys and fetch metadata once
+  all_keys <- unique(unlist(x[keycol], use.names = FALSE))
+  all_keys <- all_keys[!is.na(all_keys) & nzchar(all_keys)]


all_keys is filtered with nzchar(all_keys), but unlist(x[keycol]) can yield non-character (e.g. factor/integer64) depending on the input data.frame. nzchar() errors on non-character vectors. Coerce to character before nzchar (or use a type-agnostic empty check) to make cf_add_meta robust to common data.frame column types.

Suggested change

all_keys <- all_keys[!is.na(all_keys) & nzchar(all_keys)]

all_keys_chr <- as.character(all_keys)

all_keys <- all_keys_chr[!is.na(all_keys_chr) & nzchar(all_keys_chr)]

* the suggestions were poor because partial string matching was not enabled. * For example opic went to manc not opticlobe because of the length * also added package prefix for namespace issue

* also fixes new test error

jefferis added 3 commits March 2, 2026 07:11

Improve cf_add_meta missing key message and add test

2e450f1

* nb this replaces a bad commit from claude

jefferis requested a review from Copilot April 1, 2026 07:12

Copilot started reviewing on behalf of jefferis April 1, 2026 07:12 View session

Copilot AI reviewed Apr 1, 2026

View reviewed changes

jefferis added 4 commits April 1, 2026 08:34

add missing doc file

ffeb026

unrelated: partial string matching for datasets

9622d91

* the suggestions were poor because partial string matching was not enabled. * For example opic went to manc not opticlobe because of the length * also added package prefix for namespace issue

better limits on dataset partial matching

41ded20

* also fixes new test error

add cf_add_meta to pkgdown

ffc4252

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New cf_add_meta function#55

New cf_add_meta function#55
jefferis wants to merge 7 commits into
masterfrom
feature/cf-add-meta

jefferis commented Apr 1, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Copilot AI Apr 1, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	#' partners <- cf_partners(cf_ids(hemibrain='DA2_lPN'), threshold=10, details=FALSE)
	#' partners <- cf_partners(cf_ids(hemibrain='DA2_lPN'), threshold=10, details='neither')

	all_keys <- all_keys[!is.na(all_keys) & nzchar(all_keys)]
	all_keys_chr <- as.character(all_keys)
	all_keys <- all_keys_chr[!is.na(all_keys_chr) & nzchar(all_keys_chr)]

Conversation

jefferis commented Apr 1, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants