Skip to content

fix(data): replace infinite variable bounds with NA - #34

Merged
dlebauer merged 2 commits into
PecanProject:mainfrom
ANAMASGARD:fix/issue-29-variable-bounds
Aug 12, 2026
Merged

fix(data): replace infinite variable bounds with NA#34
dlebauer merged 2 commits into
PecanProject:mainfrom
ANAMASGARD:fix/issue-29-variable-bounds

Conversation

@ANAMASGARD

Copy link
Copy Markdown
Contributor

Description

Replaces misleading Infinity and -Infinity placeholders in the variables dataset’s min and max fields with
missing values (NA).

This preserves meaningful finite bounds. For example:

  • plant_functional_type now has min = NA and max = NA;
  • root_biomass retains min = 0 and now has max = NA.

The packaged dataset and Frictionless metadata were regenerated from the corrected source CSV.

Related Issue(s)

Closes #29

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Data update (changes to datasets)

Checklist

  • I have run devtools::check() with no errors or warnings
  • I have updated NEWS.md if applicable
  • I have updated documentation if applicable
  • I have added/updated tests if applicable

Data Changes (if applicable)

  • Data rebuilt from source using data-raw/make-data.R
  • datapackage.json updated
  • Row counts verified

Verification

  • Reproduced the original failure: 1,553 infinite placeholders in the source and packaged dataset.
  • Verified the corrected source CSV and installed package contain no infinite bounds.
  • Confirmed categorical bounds are NA and valid finite bounds, such as root_biomass მინ = 0, are preserved.
  • devtools::test() passed: 21 passed, 0 failures/warnings/skips.
  • devtools::check(error_on = "warning") passed.
  • git diff --check passed.
image --- image

Additional Notes

datapackage.json now declares variables.min and variables.max as numeric fields. Its generated created date was
updated during the data rebuild.

@dlebauer dlebauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for updating these - a simple but useful fix.

And, thank you for updating the test. But, now that the schema requires these fields to be numeric, the test could use is.infinite. There isn't a huge chance that this would occur, but please change the regression test to use expect_all_false(is.infinite(variables$min) - note that the _all_ vectorizes the expectation and thus the need for any.

This also brings up a related question that should be punted to a new issue ... the python frictionless package would make a lot of this testing of column names, types, etc much easier with frictionless validate datapackage.json.

Comment thread tests/testthat/test-data.R Outdated

@dlebauer dlebauer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks great, thanks!

@dlebauer
dlebauer merged commit 0a81116 into PecanProject:main Aug 12, 2026
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.

Where variable min / max == +/- Inf, set to NA

2 participants