Export metadata and feature data tables

[epbrenner]

Testing:

For this PR, I have included bundled test data in /inst/extdata/Campy_testdata.zip. Extract this into a single directory of your choosing. This bundled data is not intended to the ultimate example data for final package release.

For data_curation.R:
Run
prepareGenomes("Staphylococcus argenteus")
then
exportTables("data/Staphylococcus_argenteus/Sar.duckdb")

For data_processing.R:
Unzip the Campy test data first, then run
exportProcessedData("inst/extdata/Campy_testdata/Cco_parquet.duckdb")

There are several thrilling parameters to fiddle around with.

What this does:

Adds the ability to export basic stats into human readable CSVs from data_curation.R workflow.
Adds the ability to export all the processed feature data into a variety of formats from data_processing.R workflow.

What's the point?

Makes amRdata more independently useful to people even if they don't run the full amR suite. For example, you could export feature matrices for custom modeling workflows outside of ours, or just get an idea of what distributions of different QC and AST labels are across different taxa.

[jananiravi]

Few quick comments. Will test it out tomorrow.

[jananiravi]

Few quick comments. Will test it out tomorrow.

@epbrenner prepareGenomes + exportTables --> they run locally with Sar.

[jananiravi]

After running the campy --> metadata.csv

what's TET doing in resistant_classes? genome_drug.resistance_phenotype marks them as susceptible. @epbrenner @AbhirupaGhosh

[AbhirupaGhosh]

After running the campy --> metadata.csv

what's TET doing in `resistant_classes`? genome_drug.resistance_phenotype marks them as susceptible. @epbrenner @AbhirupaGhosh

resistant_classes is a column that adds all the drug classes the genome is resistant to. If there are more than 1, say TET and FLQ, it will show TET_FLQ and it shows susceptible to the genomes that are resistant to none.

[jananiravi]

is this going to be merged to qc_filtering and then qc_filtering --> main? that might be helpful to avoid duplicate reviews.

[eboyer221]

When I tested this, it looks like genome IDs such as 263.1, 263.10, and 263.100 are being written to the CSV as plain numbers rather than text, since they happen to look numeric. When I read that export back (with readr::read_csv(), pandas, or Excel's defaults) all three collapsed to the same value, since trailing zeros after a decimal don't count (263.1 and 263.10 parse as numerically identical). So the three distinct genomes merged under one ID, and it'll happen for any taxon with more than ~9 genomes. I also tried forcing quotes around every value (quote = "all") to see if that would prevent it, but the collision still happened on read. Quoting only protects against commas breaking the file format, it doesn't tell the reader "treat this as text."

This affects every genome_id-keyed table both export functions produce (genome_data, metadata, amr_phenotype_wide, gene_count, etc.).
Two options I see: (1) add a clear warning telling users to force col_types/dtype=str on ID columns when reading CSV/TSV back, or (2) recommend Parquet as the default/safe export format for programmatic reuse, since arrow preserves the real column type - I verified this did not occur with Parquet.

[epbrenner]

Oh I have to submit my own review to my own PR to actually post these comments, huh? Oops.

[jananiravi]

When I tested this, it looks like genome IDs such as 263.1, 263.10, and 263.100 are being written to the CSV as plain numbers rather than text, since they happen to look numeric. When I read that export back (with readr::read_csv(), pandas, or Excel's defaults) all three collapsed to the same value, since trailing zeros after a decimal don't count (263.1 and 263.10 parse as numerically identical). So the three distinct genomes merged under one ID, and it'll happen for any taxon with more than ~9 genomes. I also tried forcing quotes around every value (quote = "all") to see if that would prevent it, but the collision still happened on read. Quoting only protects against commas breaking the file format, it doesn't tell the reader "treat this as text."

This affects every genome_id-keyed table both export functions produce (genome_data, metadata, amr_phenotype_wide, gene_count, etc.).
Two options I see: (1) add a clear warning telling users to force col_types/dtype=str on ID columns when reading CSV/TSV back, or (2) recommend Parquet as the default/safe export format for programmatic reuse, since arrow preserves the real column type - I verified this did not occur with Parquet.

@epbrenner @AbhirupaGhosh didnt we have this fix in already?

[epbrenner]

When I tested this, it looks like genome IDs such as 263.1, 263.10, and 263.100 are being written to the CSV as plain numbers rather than text, since they happen to look numeric. When I read that export back (with readr::read_csv(), pandas, or Excel's defaults) all three collapsed to the same value, since trailing zeros after a decimal don't count (263.1 and 263.10 parse as numerically identical). So the three distinct genomes merged under one ID, and it'll happen for any taxon with more than ~9 genomes. I also tried forcing quotes around every value (quote = "all") to see if that would prevent it, but the collision still happened on read. Quoting only protects against commas breaking the file format, it doesn't tell the reader "treat this as text."
This affects every genome_id-keyed table both export functions produce (genome_data, metadata, amr_phenotype_wide, gene_count, etc.).
Two options I see: (1) add a clear warning telling users to force col_types/dtype=str on ID columns when reading CSV/TSV back, or (2) recommend Parquet as the default/safe export format for programmatic reuse, since arrow preserves the real column type - I verified this did not occur with Parquet.

@epbrenner @AbhirupaGhosh didnt we have this fix in already?

Yeah, relatively easy solution, just requires special handling of the columns of interest. Patched in the latest commit.

[jananiravi]

minor edits, comments added. yet to run line-by-line; that's the reason I didn't approve yet, but looks good.

[jananiravi]

Need this special treatment for anything else? accessions that end in a.x as well? (all of them?)

[epbrenner]

I'll ask around, but I don't think we need to worry about other fields in the trailing zero context.

Edit: Correction, we don't currently need to worry about other fields. If we update to use additional fields, this set may need to be expanded, but this catches our known issues.

[jananiravi]

@eboyer221 Test breakage (for this and newly edited funcs): this rename isn't reflected in tests/testthat/test-apply-metadata-qc.R

[jananiravi]

When build_amr_wide() returns NULL (no AMR source table, or required columns missing — see its own return(NULL) branches a few lines above), this line feeds NULL straight into .preserve_export_id_text(), silently produces a 0×0 tibble, not NULL. That will likely break downstream is.null(phenotype_wide) guards meant to skip AMR export cleanly (the next branch around line ~2035, and the append-mode check around line ~2059) — instead of skipping with the "no AMR source table found" message, it'll silently write an empty amr_phenotype_wide.csv/.parquet/etc.

Maybe only call .preserve_export_id_text() on build_amr_wide()'s result if it's non-NULL, e.g. wrap the assignment in an if (!is.null(...)).

Note for later beyond this PR -- to check new edited returns map back nicely to downstream calls.

[jananiravi]

Duplicated export-format validation/warning block — exportTables() (R/data_curation.R, ~L2423-2450) and exportProcessedData() (R/data_processing.R, ~L1858-1885) both carry an identical copy of the format-validation (allowed_formats, unknown_formats check) and the trailing-zero CSV/TSV/XLSX warning message. Not a bug, just flagging as a "worth extracting to a shared internal helper" comment if you want to leave it — your call whether it's worth the churn right now. [I think the duplicated definitions were mentioned before either for the export functions or something else.]