Edit rescaled columns and add unit tests #337
Merged
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.
This PR addresses comment (4) in #303 (comment) and for split catchments, only uses the rescaled characteristic values for columns with requested statistics "area-weighted mean" and "sum." I suggest this change because I think if a user requests "min," they expect to get the minimum value from the COMIDs that comprise the aggregated
id
(and not the minimum rescaled value).Using the same example as linked above, here's what the output looks like for characteristic
CAT_ELEV_MIN
(previously,CAT_ELEV_MIN_min = 88.5
forid = 10024048
because 88.5 was the minimum rescaled value).I've also added some unit tests to test the rescale calculations for aggregated and split catchments. These tests aren't exhaustive but help me think about how we rescale characteristics for split catchments. I realize that the doc for
rescale_catchment_characteristics
contains a note of caution about the rescaling (copied below), so let me know if you think these changes are out of scope.