Match attribute groups by whole name, not by word, when looking up valid terms
Maintainers usually reply within 1 day
Nobody has claimed this yet.
Assessment
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Newbie friendliness
- 57/100
Research direction
Start with .validAttributes() in R/physiologies.R and the multistate Attribute_value check in tests/testthat/test-importBugphyzz.R; then review inst/scripts/stop_on_unmapped.R and taxPPro's filterDataDiscrete() so the checks and export filter agree. Check whether any shape rows in the sheets or sources use endospore, branched, or cell chain. Done when group matching uses exact names across all four locations, tests confirm shape and halophily exclude unrelated terms, and the export still handles current source rows.
Written by the indexing model from the issue text.
Description
The valid terms for a physiology are looked up in inst/extdata/attributes.tsv by searching for the physiology name as a whole word in attribute_group (\b<name>\b). A physiology whose name is a whole word inside another group name therefore also gets that group's terms.
Two cases exist now:
shapealso accepts three terms that belong only tospore shape:endospore,branched,cell chain.halophilyalso accepts the terms ofhalophily class(halophilic,halotolerant,non-halophilic), added in #298.
Neither does harm today. halophily is a range attribute, so it is never checked against the vocabulary, and no shape source uses those three terms. The risk is a later discrete physiology whose wrong values pass the vocabulary check unnoticed.
Proposed fix
Match the whole group name: split attribute_group on ; and test for equality, in place of the regex.
Where the match is made
- bugphyzz,
.validAttributes()inR/physiologies.R. - bugphyzz,
tests/testthat/test-importBugphyzz.R, theAttribute_valuecheck for multistate attributes (grepl(unique(x$Attribute), attribute_group), with no word boundary at all). - bugphyzzExports,
stop_on_unmapped()ininst/scripts/stop_on_unmapped.R. - taxPPro,
filterDataDiscrete(), which decides the rows the export keeps.
The four should change together, so that the check in bugphyzz and the filter in the export keep agreeing. A test that .validAttributes("halophily") excludes the class terms and that .validAttributes("shape") excludes endospore would pin it.
Check before changing
With an exact match, shape loses the three terms above. Confirm that no shape row in the sheets or sources uses them, otherwise the export would stop on unmapped values. On the current attributes.tsv these two physiologies are the only ones whose valid terms change.
Raised in the review of #298.
Written with Claude Code.
- Dominant language
- R
- Stars
- 6
- Forks
- 6
- Avg merge
- 3h 17m
- Merged PRs (30d)
- 20
Getting set up
- No Dockerfile or Docker Compose file
- No pull request template
- Read the contributing guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
More from waldronlab/bugphyzz
-
Difficulty 2/5 1-3 hours Newbie friendliness 68/100
waldronlab/bugphyzz#301 ·
Maintainers usually reply within 1 day
-
Difficulty 2/5 1-3 hours Newbie friendliness 65/100
waldronlab/bugphyzz#281 · 1 comment ·
Maintainers usually reply within 1 day
-
Difficulty 3/5 1-2 days Newbie friendliness 77/100
waldronlab/bugphyzz#300 ·
Maintainers usually reply within 1 day
-
Difficulty 5/5 Over a week Newbie friendliness 35/100
waldronlab/bugphyzz#296 ·
Maintainers usually reply within 1 day
-
Difficulty 5/5 Over a week Newbie friendliness 28/100
waldronlab/bugphyzz#295 ·
Maintainers usually reply within 1 day
All issues in waldronlab/bugphyzz
Similar issues
-
Difficulty 1/5 Under an hour Newbie friendliness 78/100
pharmaverse/teal.picks#147 · 1 reaction ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 75/100
pharmaverse/admiralonco#367 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 67/100
nflverse/nflverse-data#107 ·
-
Difficulty 2/5 1-3 hours Newbie friendliness 62/100
-
Difficulty 2/5 1-3 hours Newbie friendliness 78/100
r-lib/tree-sitter-r#209 ·