Split cleanData and add Parquet exports by AbhirupaGhosh · Pull Request #11 · JRaviLab/amRdata

AbhirupaGhosh · 2026-02-19T15:55:20Z

Rename the original cleanData to cleanMetaData and add roxygen skeleton. Introduce a writeCompressedParquet helper and export cleaned metadata, AMR phenotype, genome data and original metadata to compressed Parquet files, then create a separate DuckDB (parquet-backed) with views for metadata, amr_phenotype, genome_data and original_metadata. Reintroduce a new cleanData function focused on feature matrices (genes/proteins/domains/etc.) that writes feature tables to Parquet and creates corresponding views; remove duplicated metadata parquet exports from the feature-matrix flow. Minor whitespace and path-handling adjustments to normalize paths and ensure output directories exist.

Description

What kind of change(s) are included?

Feature (adds or updates new capabilities)
Bug fix (fixes an issue).
Enhancement (adds functionality).
Breaking change (these changes would cause existing functionality to not work as expected).

Checklist

Please ensure that all boxes are checked before indicating that this pull request is ready for review.

I have read and followed the CONTRIBUTING.md guidelines.
I have searched for existing content to ensure this is not a duplicate.
I have performed a self-review of these additions (including spelling, grammar, and related).
I have added comments to my code to help provide understanding.
I have added a test which covers the code changes found within this PR.
I have deleted all non-relevant text in this pull request template.
Reviewer assignment: Tag a relevant team member to review and approve the changes.

Rename the original cleanData to cleanMetaData and add roxygen skeleton. Introduce a writeCompressedParquet helper and export cleaned metadata, AMR phenotype, genome data and original metadata to compressed Parquet files, then create a separate DuckDB (parquet-backed) with views for metadata, amr_phenotype, genome_data and original_metadata. Reintroduce a new cleanData function focused on feature matrices (genes/proteins/domains/etc.) that writes feature tables to Parquet and creates corresponding views; remove duplicated metadata parquet exports from the feature-matrix flow. Minor whitespace and path-handling adjustments to normalize paths and ensure output directories exist.

Fixed trailing zero bug, fixed FTP timeout bug (?), fixed empty files hanging downloads, fixed imbalanced genome data sets (e.g., no .fna, yes .faa, yes .gff)

AbhirupaGhosh · 2026-02-27T22:06:44Z

+    message("  amr_phenotype: ", n_amr_ids)
+    message("  genomes with 0 AMR rows: ", length(ids_zero_amr))
+    if (length(ids_zero_amr)) {
+      message("  e.g.: ", paste(utils::head(ids_zero_amr, 10), collapse = ", "))


It printed

Initial summary: targets=24421 | AMR genomes=24422 | genome_data genomes=24423 Final summary: targets : 24421 bac_data : 24421 genome_data : 24423 amr_phenotype: 24422 genomes with 0 AMR rows: 2 e.g.: , genome.genome_id

So I am guessing it is considering column name as genome id!

We can strip this out. It was for debugging originally which is why it's messy. Alternatively, if it is useful to keep a summary, I can work on some improvements for it!

I didn't find any logical impact of this misprinting. We can come back to this later. I have made an issue.

AbhirupaGhosh · 2026-02-27T23:21:17Z

+  message("FTPS pass 1 (30s timeout)")
+  future::plan(future::multisession, workers = max(1, workers_first))
+  res1 <- future.apply::future_lapply(
+    genome_ids,
+    function(gid) {
+      ok <- .ftpes_download_one(gid, out_dir,
+        connect_timeout = 10L, max_time = 30L,
+        speed_time = 30L, speed_limit = 2048L


Suggested change

message("FTPS pass 1 (30s timeout)")

future::plan(future::multisession, workers = max(1, workers_first))

res1 <- future.apply::future_lapply(

genome_ids,

function(gid) {

ok <- .ftpes_download_one(gid, out_dir,

connect_timeout = 10L, max_time = 30L,

speed_time = 30L, speed_limit = 2048L

message("FTPS pass 1 (45s timeout)")

future::plan(future::multisession, workers = max(1, workers_first))

res1 <- future.apply::future_lapply(

genome_ids,

function(gid) {

ok <- .ftpes_download_one(gid, out_dir,

connect_timeout = 10L, max_time = 45L,

speed_time = 30L, speed_limit = 2048L

AbhirupaGhosh · 2026-02-27T23:21:40Z

+  message("FTPS pass 2 (60s timeout) for failed genomes")
+  future::plan(future::multisession, workers = max(1, workers_second))
+  res2 <- future.apply::future_lapply(
+    fail_ids,
+    function(gid) {
+      ok <- .ftpes_download_one(gid, out_dir,
+        connect_timeout = 10L, max_time = 60L,
+        speed_time = 30L, speed_limit = 2048L
+      )
+      list(gid = gid, ok = ok)
+    },
+    future.seed = TRUE
+  )


Suggested change

message("FTPS pass 2 (60s timeout) for failed genomes")

future::plan(future::multisession, workers = max(1, workers_second))

res2 <- future.apply::future_lapply(

fail_ids,

function(gid) {

ok <- .ftpes_download_one(gid, out_dir,

connect_timeout = 10L, max_time = 60L,

speed_time = 30L, speed_limit = 2048L

)

list(gid = gid, ok = ok)

},

future.seed = TRUE

)

message("FTPS pass 2 (120s timeout) for failed genomes")

future::plan(future::multisession, workers = max(1, workers_second))

res2 <- future.apply::future_lapply(

fail_ids,

function(gid) {

ok <- .ftpes_download_one(gid, out_dir,

connect_timeout = 10L, max_time = 120L,

speed_time = 30L, speed_limit = 2048L

)

list(gid = gid, ok = ok)

},

future.seed = TRUE

)

Added a function to parse CD-HIT .clstr output into a long-format mapping of clusters to member feature ids. Updated database writing logic to include the new protein members table.

Minor regex change --> cleanData

Updated resistance summary calculation to read from the database and include a count of resistant classes.

AbhirupaGhosh · 2026-03-18T22:03:16Z

    dplyr::distinct() |>
    dplyr::filter(!is.na(protein_ids)) |>
    tidyr::separate_rows(protein_ids, sep = ";") |>
    dplyr::filter(!stringr::str_detect(protein_ids, "_pseudo")) |>


Suggested change

# dplyr::filter(!stringr::str_detect(protein_ids, "_pseudo")) |>

dplyr::mutate(protein_ids = gsub("_pseudo", "", protein_ids)) |>

This is one of the reasons I found that a lot of group ids don't have mapped bvbrc feature ids .

AbhirupaGhosh · 2026-04-10T20:38:34Z

+      names_to = "genome_ids",
+      values_to = "protein_ids"
+    ) |>
    dplyr::mutate(genome_ids = gsub(".PATRIC", "", genome_ids)) |>


Suggested change

dplyr::mutate(genome_ids = gsub(".PATRIC", "", genome_ids)) |>

dplyr::mutate(genome_ids = sub("\\.PATRIC\\.\\.\\..*$", "", genome_ids)) |>

eboyer221

Ran PR locally for Shigella flexneri and it worked as expected once I made the code changes suggested in this review (see inline comments below).
All expected tables present at the end of the run:

Metadata: metadata, amr_phenotype, genome_data, original_metadata
Genes: gene_count, gene_names, gene_seqs, struct
Proteins: protein_count, protein_names, protein_seqs, protein_members, genome_gene_protein
Domains: domain_count, domain_names

eboyer221 · 2026-04-15T20:07:09Z

 #'
 #' @keywords internal
 .buildProtMatrices <- function(cluster_map) {
  cluster_map[, count := 1]


The := operator is used here, but data.table is not imported into the package namespace. Currently, it is only listed under Imports: in DESCRIPTION.

When I ran runDataProcessing() locally, this resulted in an error:
[ was called on a data.table in an environment that is not data.table-aware (cedta())

CD-HIT runs, but the error comes immediately after, when R tries to parse the CD-HIT outputs into a cluster matrix.

To fix, add data.table to any R/*.R file with:

#' @importFrom data.table := NULL

Then run devtools::document() to regenerate NAMESPACE. One import covers every := call in the package, so no need to repeat it per file.

eboyer221 · 2026-04-15T22:16:52Z

@@ -1627,6 +1792,8 @@
  }
  if (isTRUE(verbose)) message("Cleaning metadata and exporting Parquet-backed views.")
  cleanData(duckdb_path = duckdb_path, path = out_dir, ref_file_path = ref_file_path)


This code is leftover from the cleanData -> cleanMetaData rename. The new cleanData() signature no longer accepts ref_file_path, so this line errors with unused argument (ref_file_path = ref_file_path). The two new calls on lines 1795–1796 already handle this correctly — this line should be deleted.

Suggested change

cleanData(duckdb_path = duckdb_path, path = out_dir, ref_file_path = ref_file_path)

AbhirupaGhosh and others added 3 commits January 28, 2026 16:31

Update regex patterns for feature ID extraction

73f7b52

Style code (GHA)

8d494a5

AbhirupaGhosh requested review from charmvang, epbrenner and jananiravi February 19, 2026 15:55

AbhirupaGhosh self-assigned this Feb 19, 2026

AbhirupaGhosh and others added 3 commits February 19, 2026 15:56

Style code (GHA)

c52aaa7

Updating download logic

4edc964

Fixed trailing zero bug, fixed FTP timeout bug (?), fixed empty files hanging downloads, fixed imbalanced genome data sets (e.g., no .fna, yes .faa, yes .gff)

Style code (GHA)

5287d1b

AbhirupaGhosh assigned AbhirupaGhosh and epbrenner and unassigned AbhirupaGhosh Feb 26, 2026

AbhirupaGhosh commented Feb 27, 2026

View reviewed changes

AbhirupaGhosh mentioned this pull request Feb 27, 2026

Summary printing after retrieveMetadata #12

Open

AbhirupaGhosh commented Feb 27, 2026

View reviewed changes

AbhirupaGhosh and others added 10 commits March 11, 2026 16:31

Add CD-HIT parsing function and update DB logic

f04d3dc

Added a function to parse CD-HIT .clstr output into a long-format mapping of clusters to member feature ids. Updated database writing logic to include the new protein members table.

Style code (GHA)

6bcddb6

Merge branch 'cleanData' into minor_regex_change

394cfd7

Merge pull request #15 from JRaviLab/minor_regex_change

9da2f23

Minor regex change --> cleanData

Refactor data processing to use filtered_metadata

918b2e0

Style code (GHA)

e85f88f

Filter genome drug resistant phenotype in data processing

3461807

Style code (GHA)

577b251

Refactor resistance summary calculation

ea4d844

Updated resistance summary calculation to read from the database and include a count of resistant classes.

Style code (GHA)

93b5b2e

AbhirupaGhosh commented Mar 18, 2026

View reviewed changes

eboyer221 self-requested a review April 8, 2026 17:43

AbhirupaGhosh commented Apr 10, 2026

View reviewed changes

eboyer221 requested changes Apr 15, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Split cleanData and add Parquet exports#11

Split cleanData and add Parquet exports#11
AbhirupaGhosh wants to merge 16 commits intomainfrom
cleanData

AbhirupaGhosh commented Feb 19, 2026

Uh oh!

AbhirupaGhosh Feb 27, 2026

Uh oh!

epbrenner Feb 27, 2026

Uh oh!

AbhirupaGhosh Feb 27, 2026

Uh oh!

AbhirupaGhosh Feb 27, 2026

Uh oh!

AbhirupaGhosh Feb 27, 2026

Uh oh!

AbhirupaGhosh Mar 18, 2026

Uh oh!

AbhirupaGhosh Apr 10, 2026

Uh oh!

eboyer221 left a comment •

edited

Loading

Uh oh!

eboyer221 Apr 15, 2026

Uh oh!

eboyer221 Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


	# dplyr::filter(!stringr::str_detect(protein_ids, "_pseudo")) \|>
	dplyr::mutate(protein_ids = gsub("_pseudo", "", protein_ids)) \|>

	dplyr::mutate(genome_ids = gsub(".PATRIC", "", genome_ids)) \|>
	dplyr::mutate(genome_ids = sub("\\.PATRIC\\.\\.\\..*$", "", genome_ids)) \|>

Conversation

AbhirupaGhosh commented Feb 19, 2026

Description

What kind of change(s) are included?

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eboyer221 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

eboyer221 left a comment •

edited

Loading