Phase 1: Make cf_ids the single source of ID resolution#50
Closed
Phase 1: Make cf_ids the single source of ID resolution#50
Conversation
- Change cf_ids expand default to TRUE - Add 'expanded' attribute to cidlist objects to track resolution state - Add check_expanded() to verify IDs have been resolved before use - cf_partners and cf_meta now check for expanded IDs instead of calling expand_ids - c.cidlist checks inputs are expanded and propagates the attribute - Remove redundant fanc_ids()/banc_ids() calls from .fanc_partners() and .banc_partners() This ensures all query resolution happens in cf_ids, making it the single entry point for ID resolution. Downstream functions receive only resolved numeric IDs, preparing for Phase 3 local cache support.
5 tasks
- Remove ::: call to flywire_meta in flywire2.R (use direct call within package) - Add .rhubarb_ids() function to test setup for rhubarb dataset - Fix banc test to skip when bancr is not properly installed - Fix triple_connection_table to expand IDs before passing to cf_partners - Update badrhubarb test to check for correct error messages
- Skip malecns-dependent test if malecns not installed - Remove specific error regex for badrhubarb (just expect any error)
malecns was in Enhances which isn't installed during R CMD check. Moving to Suggests makes it available for tests.
Revert DESCRIPTION change (keep malecns in Enhances). Add malecns to check-r-package extra-packages so it's available during tests.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
cf_idsexpand default to TRUEcheck_expanded()to verify IDs have been resolved before usecf_partnersandcf_metanow check for expanded IDs instead of callingexpand_idsc.cidlistchecks inputs are expanded and propagates the attributefanc_ids()/banc_ids()calls from.fanc_partners()and.banc_partners()This ensures all query resolution happens in
cf_ids, making it the single entry point for ID resolution. Downstream functions receive only resolved numeric IDs, preparing for Phase 3 local cache support wherecf_idswill decide whether to query locally or remotely.Test plan
cf_ids(hemibrain='MBON01')expands by defaultcf_ids(..., expand=FALSE)setsattr(result, 'expanded') = FALSEcf_partnerserrors on unexpanded queriescf_partnersworks with raw numeric IDsc(cf_ids(...), cf_ids(...))works and propagates expanded attribute