Skip to content

Bug fix: units produce a crash with multispec data#930

Merged
js3110 merged 8 commits intomainfrom
925-bug-units-produce-a-crash-with-multispec-data
Jan 28, 2026
Merged

Bug fix: units produce a crash with multispec data#930
js3110 merged 8 commits intomainfrom
925-bug-units-produce-a-crash-with-multispec-data

Conversation

@js3110
Copy link
Copy Markdown
Collaborator

@js3110 js3110 commented Jan 23, 2026

Issue

Closes #925

Description

Multispec data was crashing due to AMOUNTU column having NA and non NA units, which had changes in processing after #907
This PR updates the units table build to allow NA units in AMOUNTU, as they are handled differently to AVALU and the others. AMOUNTU can have NA units if there is no VOLUME value.

Definition of Done

  • App does not crash with multispec data

How to test

Try to run NCA with multispec data in testthat folder

Contributor checklist

  • Code passes lintr checks
  • Code passes all unit tests
  • New logic covered by unit tests
  • New logic is documented
  • App or package changes are reflected in NEWS
  • Package version is incremented
  • R script works with the new implementation (if applicable)

@js3110 js3110 linked an issue Jan 23, 2026 that may be closed by this pull request
1 task
@js3110 js3110 marked this pull request as ready for review January 23, 2026 16:32
Copy link
Copy Markdown
Collaborator

@Gero1999 Gero1999 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed the original error message mentioned unrelated parameters to AMOUNTU:

Warning: Error in pknca_unit_conversion: Units are provided for some but not all parameters; missing for: adj.r.squared, clast.pred, half.life, lambda.z, lambda.z.n.points, lambda.z.time.first, lambda.z.time.last, r.squared, span.ratio, tlast, tmax
This error can be converted to a warning using `PKNCA.options(allow_partial_missing_units = TRUE)`

But I guess this data-case solution made it work for the testing data... I will approve

@Gero1999
Copy link
Copy Markdown
Collaborator

For future reference, lets include the original issue that produced this: #907

Comment thread tests/testthat/test-PKNCA.R
@js3110 js3110 requested a review from m-kolomanski January 26, 2026 13:20
@js3110 js3110 merged commit 308c3ae into main Jan 28, 2026
10 checks passed
@js3110 js3110 deleted the 925-bug-units-produce-a-crash-with-multispec-data branch January 28, 2026 09:13
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.

Bug: units produce a crash with multispec data

4 participants