Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

format_ae_listing(): AEDOSDUR parser errors on repeated Y/M/D designator and is not vectorized

Open
#160 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
65/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Active
Tech stack
r
Domain
data

Research direction

The bug is in R/ae_listing.R lines 458-501. Start by reading the format_ae_listing() function and the loop that parses AEDOSDUR. Understand how gregexpr works and why a repeated designator causes an error. Look into vectorized regex solutions using sub or regmatches. Check related issues #138 and #148 for context. A fix should replace the loop with a vectorized parser and handle malformed input gracefully. Test with sample data containing repeated 'Y', 'M', or 'D' letters.

Written by the indexing model from the issue text.

Description

bug

The Total_Dose_on_Day_of_AE_Onset derivation in format_ae_listing() parses the ISO-8601-like AEDOSDUR string one row at a time (R/ae_listing.R, around lines 458-501), branching on unlist(gregexpr("Y", ...)) > 0 etc.:

for (i in 1:length(res[["AEDOSDUR"]])) {
  if (unlist(gregexpr("Y", res[["ymd"]][i])) > 0) { ... }
  if (unlist(gregexpr("M", res[["ymd"]][i])) > 0) { ... }
  if (unlist(gregexpr("D", res[["ymd"]][i])) > 0) { ... }
}

Bug. gregexpr() returns every match position. If a value contains a designator letter more than once, the result has length > 1, and if (... > 0) raises a hard error on R >= 4.2:

r <- unlist(gregexpr("Y", "1Y2Y3D"))   # c(2, 4)
if (r > 0) "x"
#> Error: the condition has length > 1

Valid ISO 8601 durations use each designator once, so this is latent, but any malformed / dirty AEDOSDUR value triggers it. Also note gregexpr matches literally, so an unexpected letter elsewhere in the string could match.

Secondary (readability / performance). The loop calls gregexpr()/substring() roughly nine times per row for a task a single vectorized regex handles. A capture-group approach (e.g. sub("^(?:([0-9]+)Y)?(?:([0-9]+)M)?(?:([0-9]+)D)?$", ...) or regmatches) removes both the loop and the length>1 hazard, and pluralizes units cleanly.

Related cleanup: #138, #148.

Dominant language
R
Stars
22
Forks
6
Avg merge
1d 13h
Merged PRs (30d)
10

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from Merck/forestly

All issues in Merck/forestly

Similar issues

More R issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.