Skip to content

refactor: move data ingestion (readers, checks, GEO, single-cell) to playbase.ingest - #528

Open
ESCRI11 wants to merge 4 commits into
fantafrom
feat/playbase-ingest
Open

ESCRI11 wants to merge 4 commits into
fantafrom
feat/playbase-ingest

Conversation

@ESCRI11

@ESCRI11 ESCRI11 commented Sep 21, 2026

Copy link
Copy Markdown
Contributor

Moves data ingestion to the new leaf package bigomics/playbase.ingest. playbase calls it via playbase.ingest::, with no re-exports (same as playbase.preprocess).

Moved: pgx-read.R, pgx-check.R, pgx-getgeo.R, read.gmt, h5.readMatrix, trimsame*, seurat2pgx, pgx.read_singlecell_counts, example_file, PGX_CHECKS, fixtures and tests.

Not moved (on purpose, for now): epigenomics ingestion (IDAT readers in playbase.epigenetics).

test-reexports.R now checks that playbase doesn't redefine anything in playbase.ingest. Downstream playbase::read_* / GEO calls must switch to playbase.ingest::.

🤖 Generated with Claude Code

ESCRI11 and others added 2 commits September 21, 2026 10:45
Moves R/pgx-read.R, R/pgx-check.R, read.gmt, h5.readMatrix,
first_feature, example_file, PGX_CHECKS (data + builder), inst/extdata
and the reader tests to bigomics/playbase.ingest. playbase declares it
in Imports/Remotes and calls it through explicit playbase.ingest::,
with no re-exports, matching the playbase.preprocess boundary.

- pgx.createFromFiles, pgxinfo.*, mofa.exampledata, the GEOquery reader
  and pgx.getGeneFamilies now call playbase.ingest:: (the nested
  read.gmt copy in pgx.getGeneFamilies is dropped)
- bit64 and OlinkAnalyze leave Imports
- unittest image installs playbase.ingest before R CMD INSTALL
- test-reexports.R asserts playbase neither exports nor defines any
  playbase.ingest export

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ngest

R/pgx-getgeo.R, trimsame/trimsame.ends/trimsame0, seurat2pgx and
pgx.read_singlecell_counts now live in playbase.ingest (nothing else in
playbase called them). ArrayExpress, Biobase, biomaRt, GEOquery, msa,
R.utils and recount leave Imports.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@ESCRI11 ESCRI11 changed the title refactor: move readers and input checks to playbase.ingest refactor: move data ingestion (readers, checks, GEO, single-cell) to playbase.ingest Sep 21, 2026
@phisanti

Copy link
Copy Markdown
Contributor

Clanker found few things:

  1. These packages should be on Imports:
Biobase::  16 uses 
org.Hs.eg.db  6 
rhdf5::    18 uses 
AnnotationDbi 4 
GEOquery:: 10 uses 
recount::     3 
playbase.preprocess
  1. seurat2pgx signature has changed silently
fanta   seurat2pgx(obj, do.cluster = FALSE, organism)
ingest  seurat2pgx(obj)

organism gone, pgx$genes <- ngs.getGeneAnnotation(...) gone, the whole do.cluster → pgx.clusterSamples() branch gone. Architecturally forced (both helpers stayed in playbase; the leaf can't depend back), but it's exported, unmentioned in the PR body, and any caller passing organism= now errors. No callers anywhere, so latent. Needs an explicit sign-off. The @CountS slot → Seurat::GetAssayData swap in the same function is an improvement.

  1. PGX_CHECKS now has two owners

playbase.ingest/data-raw/PGX_CHECKS.csv (moved here, byte-identical) and playbaseCore::PGX_CHECKS (pinned separately, consumed by r-service/.../svc_upload_validate.R:47). Whichever is canonical, nothing enforces it. Drift risk worth a bead.

@phisanti phisanti left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Minor comments

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