Skip to content

fix: resolve composite event columns via faers_get, support all MedDRA hierarchy levels - #29

Merged
MadDERt merged 1 commit into
WangLabCSU:develfrom
MadDERt:fix/phv-composite-bug
Sep 28, 2026
Merged

MadDERt merged 1 commit into
WangLabCSU:develfrom
MadDERt:fix/phv-composite-bug

Conversation

@MadDERt

@MadDERt MadDERt commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

Description

faers_phv_composite() / faers_phv_signal_composite() were broken for
MedDRA hierarchy event types and bypassed the duckdb backend. This PR fixes
the event-column resolution and hardens the input validation.

The core fix: .identify_event_set_patients() now resolves event columns
via faers_get(object, "reac"), which attaches the MedDRA hierarchy columns
on both the memory and the duckdb backends — the same resolution path used
by faers_counts() and faers_phv_scan().

Problems fixed

  1. Hierarchy event types crashed — .event_type = "soc_name" aborted
    with object 'soc_name' not found on both backends, because the raw
    reac table lacks hierarchy columns. Now works for any standardized
    reac column (pt, meddra_code, meddra_pt, and all MedDRA hierarchy
    columns such as soc_name / hlgt_name), with a helpful error message
    listing the available columns.
  2. The duckdb backend was bypassed — db_collect(con, "reac") pulled the
    whole table into memory; removed in favor of faers_get().
  3. Dead code that crashed on duckdb-backed .full objects — an unused
    faers_counts() call went through counts_db() and failed with a SQL
    binder error; removed.
  4. De-duplication is now enforced (faers_dedup() required), consistent
    with faers_phv_scan().
  5. Undeclared assertthat usage replaced with the internal assert_()
    helper (same style as scan).
  6. A no-op setnames() line was removed.

Testing

  • New tests/testthat/test_signal-eventset.R (the file did not exist):
    26 assertions — soc_name/hlgt_name regression, character & function
    event sets, 2×2 conservation, .object2 comparison mode, the
    de-duplication guard, and memory ↔ duckdb identical() parity
  • Full suite: 361 passing; the only failures are the pre-existing
    test_meta.R network/cache environment items (unchanged baseline)
  • R CMD check: no new issues introduced

Notes

  • Behavior change: de-duplicated objects are now required (consistent with
    faers_phv_scan()); raw pt mode output is otherwise unchanged
  • Follow-up (not in this PR): duckdb SQL pushdown so .full composite calls
    filter in-database instead of materializing the table

@ShixiangWang

Copy link
Copy Markdown
Contributor

@MadDERt 如果是ai工作生成的,确认代码你都自己阅读过,然后check过了你自己可以直接合并。

后面的pr都这样的标准处理

@MadDERt
MadDERt merged commit 65a5567 into WangLabCSU:devel Sep 28, 2026
11 checks passed
@MadDERt
MadDERt deleted the fix/phv-composite-bug branch September 28, 2026 06:06
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