Skip to content

Commit fc16cd3

Browse files
dfalsterclaude
andauthored
Refactor match_taxa() and expand match-step test coverage (#283)
Pins `match_taxa()` behaviour with tests, then refactors the matcher against them. **#287 has been merged into this branch**, so this PR now carries both halves of the work and is the single merge to `master`. ## Part 1 — safety net (this branch's original scope) - **Fix** malformed `aligned_reason` for fuzzy genus-level `aff.`/`affinis` matches (`match_06b`/`06c`/`06d`), which appended the date without a separating ` (` — output was `...genus-rank2026-..)`. - **Extend** `test_matches_alignments_updates.csv` with inputs for the 7 match steps no benchmark previously reached (`03e`, `04e`, `06e`, `08e`, `12g`, `12h`, `12i`); now covers all 54. Refreshed the previously-unused `alignment_code` column to real values. - **Assert** `alignment_code` in the alignment regression test (was selected but never checked — which is why the reason bug hid). - **Add** `test-match_branches.R`: dark-branch resolution, `aligned_reason` well-formedness, and a full-output snapshot (`_snaps/match_branches.md`, date-normalised). ## Part 2 — the refactor (#287, reviewed by @ehwenk) `R/match_taxa.R` was ~2150 lines of near-identical blocks, each repeating: build a logical index → `match()` a resource table → `mutate()` seven columns → `redistribute()` → early-return. Five helpers now carry that shape: `apply_match()`, `match_reference_name()`, `genus_sp_name()`/`higher_rank_name()`, `fuzzy_match_column()`, `drop_scratch()`. Code lines excluding comments and blanks: **1606 → 766**; total file 2156 → 1192, with the per-branch taxonomic comments kept verbatim. `apply_match()` appends ` (<date>)` to `aligned_reason` centrally, so the separator that broke in Part 1 can no longer drift between match steps. **One behaviour fix:** `align_taxa(full = TRUE)` leaked the internal `identifier_string`, `identifier_string2` and `aligned_name_tmp` columns whenever every input aligned before the last match step ran — cleanup only sat on the final return, not the 53 early ones. Now runs on every exit path. **Review follow-ups applied:** helper arguments are named for the output columns they populate (`aligned_reason`, `alignment_code`, `taxonomic_dataset`, `taxon_rank`) so they stay distinct from `update_reason` on the update side; `column` → `name_type`; and the matcher is described as a linear series of "match steps" rather than "branches". ## Verification - All 54 alignment codes present and in the same order. - `align_taxa(full = TRUE)` byte-identical to pre-refactor across all 26 columns for the 253 benchmark + dark-step + ad-hoc names, under four option combinations. Only difference is the three leaked columns disappearing from the early-exit case. - Full suite: **175 passed, 0 failed, 0 skipped**, with `_snaps/match_branches.md` unmodified — the safety net from Part 1 held across the whole refactor. - `R CMD check --as-cran`: 0 errors, 0 warnings. (2 notes, both pre-existing on `master`: `AGENTS.md` and `CITATION.cff` are not in `.Rbuildignore`.) ## Not fixed here Four `aligned_reason` strings contain stray double spaces (`identification but`, `same genus, but`, `hybrid, but`), preserved exactly since fixing them changes user-facing output. There is also more repetition left to collapse in the fuzzy genus/family steps — noted in `AGENTS.md` as the next target. Closes #279. Related: #281 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 8aafec6 commit fc16cd3

8 files changed

Lines changed: 1305 additions & 2064 deletions

File tree

AGENTS.md

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,33 @@ The user-facing pipeline is **align → update**:
1313
- `align_taxa()` ([R/align_taxa.R](R/align_taxa.R)) — standardises input names
1414
and finds the best APC/APNI alignment. Builds a `taxa` list with `tocheck` and
1515
`checked` tibbles, then delegates to `match_taxa()`.
16-
- `match_taxa()` ([R/match_taxa.R](R/match_taxa.R)) — **the core matcher, ~2150
17-
lines.** It runs ~54 sequential match branches (`match_01a``match_12i`),
18-
each: compute a logical index `i`, `match()` against a resource table, `mutate()`
19-
the matched rows with `aligned_name`/`taxon_rank`/`taxonomic_dataset`/
20-
`aligned_reason`/`alignment_code`, then `redistribute()` checked rows out of
21-
`tocheck` and early-return when `tocheck` is empty. Branches are heavily
22-
copy-pasted — see "Known issues".
16+
- `match_taxa()` ([R/match_taxa.R](R/match_taxa.R)) — **the core matcher.** It
17+
runs ~54 sequential match steps (`match_01a``match_12i`). Each builds a
18+
logical index `i` of the rows it can resolve, calls `apply_match()` to stamp
19+
those rows with `aligned_name`/`taxon_rank`/`taxonomic_dataset`/
20+
`aligned_reason`/`alignment_code` and `redistribute()` them out of `tocheck`,
21+
then early-returns when `tocheck` is empty. **The order of the match steps is
22+
the algorithm** — do not reorder without re-running the alignment benchmarks.
23+
It is a linear series of successive checks, not a branching structure; say
24+
"match step", not "branch".
25+
26+
Match-step helpers, all at the bottom of the same file. Their arguments are
27+
named for the output columns they populate (`taxonomic_dataset`, `taxon_rank`,
28+
`aligned_name`, `aligned_reason`, `alignment_code`) — keep it that way, since
29+
the update side of the package has its own `update_reason`:
30+
- `apply_match()` — the shared match-step tail. Appends ` (<date>)` to
31+
`aligned_reason` centrally, so that separator can no longer be mistyped
32+
per-step (it was, three times).
33+
- `match_reference_name()` — the shape 21 match steps share: rows whose `key`
34+
column of `tocheck` exactly matches the `name_type` column of a reference
35+
table take that reference row's canonical name and rank.
36+
- `genus_sp_name()` / `higher_rank_name()` — build `Acacia sp. [Royal NP]` and
37+
`Acacia sp. [acacia aff. dealbata; Royal NP]` respectively.
38+
- `fuzzy_match_column()` — fuzzy-match a whole column; NA inputs pass through
39+
(`fuzzy_match()` errors on NA).
40+
- `drop_scratch()` — removes `identifier_string`/`identifier_string2` on every
41+
exit path, so `full = TRUE` output is the documented column set regardless of
42+
which match step finished the job.
2343
- `update_taxonomy()` ([R/update_taxonomy.R](R/update_taxonomy.R)) — maps aligned
2444
names to currently accepted names, handling synonyms and taxonomic splits.
2545
- `load_taxonomic_resources()` ([R/load_taxonomic_resources.R](R/load_taxonomic_resources.R))
@@ -88,10 +108,10 @@ the CSV.
88108

89109
### Known issues / landmines
90110

91-
- **`match_taxa.R` duplication.** ~54 near-identical blocks. This has already bred
92-
bugs (a missing ` (` before the date in three `aff.` fuzzy branches). A helper
93-
(`apply_match(...)`) would collapse it dramatically — do it as its own PR backed
94-
by the snapshot above.
111+
- **`match_taxa.R` duplication.** Largely resolved: the ~54 near-identical blocks
112+
now go through the helpers above. Some repetition remains (the fuzzy
113+
genus/family match steps still repeat a recognisable shape), so there is more
114+
to collapse — but only ever backed by the snapshot above.
95115
- **`native_anywhere_in_australia()`**: the `is.null(resources)` guard runs *after*
96116
`create_species_state_origin_matrix()` (so offline errors instead of failing
97117
gracefully), and its `apply(..., grepl("native", x))` greps across all columns

NEWS.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
# APCalign 2.0.0
22

3+
- Fix malformed `aligned_reason` text for fuzzy genus-level `aff.`/`affinis` matches (`match_06b`/`06c`/`06d`), which previously appended the date without a separating ` (`.
4+
- `align_taxa(full = TRUE)` no longer leaks the internal `identifier_string`, `identifier_string2` and `aligned_name_tmp` columns when every name is aligned before the last match step runs. The output is now the documented set of columns in all cases.
5+
- Internal refactor of `match_taxa()`: the ~54 match steps now share helper functions rather than repeating the same block of code. Alignment output is unchanged.
36
- `native_anywhere_in_australia()` now checks for missing taxonomic resources before building the state-origin matrix, so an offline call reports the problem once instead of once per function that gives up. Its native/introduced test also now reads only the state columns, so a taxon whose name contains "native" (e.g. the `nativitatis` epithets) can no longer be misclassified.
47
- New function `synonyms_for_accepted_names()` to list synonyms for currently accepted taxon names.
58
- `load_taxonomic_resources()` now caches results in memory for the duration of the R session, so repeated calls with the same version return immediately without re-downloading or re-processing data.

R/APCalign-package.R

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,6 @@ utils::globalVariables(
8787
"taxonomic_dataset",
8888
"taxonomic_dataset_genus",
8989
"trinomial",
90-
"aligned_name_tmp",
91-
"identifier_string",
92-
"identifier_string2",
9390
"suggested_name",
9491
"update_reason",
9592
"taxon_ID_genus",

0 commit comments

Comments
 (0)