Source shared defaults from fireSenseUtils, add scanfiVersion parameter - #50
Merged
Merged
Conversation
fireSense_dataPrepFit.R:36-88 hard-coded forestedLCC, cutoffForYoungAge, nonForestCanBeYoungAge, flammabilityThreshold, fuelClassCol and igAggFactor, duplicating the same defaults fireSense_dataPrepPredict also hard-codes; the rock-nonflammable bug fixed in v1.2.0.9017 came from exactly this pattern. These now default to the corresponding fireSenseUtils::fireSense* constants (fireSenseUtils PR #105), with no change in value or behaviour. New parameter scanfiVersion (default fireSenseUtils::fireSenseSCANFIVersion, "V3") is passed to fireSenseUtils::makeFireSenseLCC() at fireSense_dataPrepFit.R:1454. fireSenseUtils:::fireSenseCovariatesCreate (line 729) is now called as `::`, since it is exported. reqdPkgs floors fireSenseUtils@development (>= 0.2.3.9062). Version 1.2.0.9018. Rmd docs re-rendered (new scanfiVersion parameter row). Verified with SpaDES.core::convertToPackage() + testthat::test_local() against a scratch library (fireSenseUtils installed from the unmerged feat/shared-module-defaults branch). Base (development): FAILED 7, ERRORS 17, SKIPPED 1, TESTS 64. Branch: FAILED 7, ERRORS 17, SKIPPED 1, TESTS 66, same failing tests as base (pre-existing, unrelated to this change). The two new tests in test-sharedDefaults.R fail on base (scanfiVersion parameter and makeFireSenseLCC() argument do not exist there) and pass on branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fireSense_dataPrepFit.R:36-88 hard-coded six parameter defaults (forestedLCC, cutoffForYoungAge, nonForestCanBeYoungAge, flammabilityThreshold, fuelClassCol, igAggFactor) that fireSense_dataPrepPredict also hard-codes separately; the rock-nonflammable bug fixed in v1.2.0.9017 came from exactly this duplication. These now default to the matching
fireSenseUtils::fireSense*constants added in fireSenseUtils#105, with the same values as before, so behaviour is unchanged. A new parameter,scanfiVersion(defaultfireSenseUtils::fireSenseSCANFIVersion,"V3"), is passed tofireSenseUtils::makeFireSenseLCC()at fireSense_dataPrepFit.R:1454.fireSenseUtils:::fireSenseCovariatesCreateat line 729 is now called with::, since it is exported. ThefireSenseUtilsfloor inreqdPkgsmoves to@development (>= 0.2.3.9062).Verified with
SpaDES.core::convertToPackage()+testthat::test_local(), fireSenseUtils installed from the unmergedfeat/shared-module-defaultsbranch into a scratch library. Base (development): 7 failed, 17 errors, 1 skipped, 64 tests. This branch: 7 failed, 17 errors, 1 skipped, 66 tests, the same failing/erroring tests as base (pre-existing, unrelated to this change). The two new tests intest-sharedDefaults.Rfail on base, because thescanfiVersionparameter and themakeFireSenseLCC()argument do not exist there, and pass on this branch.CI will fail until fireSenseUtils#105 merges, since that PR is the source of the floor version this module now requires.
🤖 Generated with Claude Code
https://claude.ai/code/session_01CwcjqqK59FmTJscyi7xUqv