Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add and fix test #137

Merged
merged 5 commits into from
Dec 12, 2024
Merged

add and fix test #137

merged 5 commits into from
Dec 12, 2024

Conversation

barneydobson
Copy link
Collaborator

apply_overrides of surfaces wasn't being called during model load. This tests and fixes.

Fixes #136

RobinMaesPrior
RobinMaesPrior reviewed Dec 11, 2024
View reviewed changes
Copy link
Collaborator

@RobinMaesPrior RobinMaesPrior left a comment

Choose a reason for hiding this comment

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

Potentially want to add a GrowingSurface override into the test?

@barneydobson
Copy link
Collaborator Author

But Woodland and Grassland are growing surfaces? maybe I misunderstand?

@RobinMaesPrior
Copy link
Collaborator

Ah, I meant in terms of a GrowingSurface specific parameter. The current parameter checked is Area, which is Surface generic. Likely fine without, but just thought I'd flag it

@barneydobson
Copy link
Collaborator Author

ahh understood - yes good suggestion

RobinMaesPrior
RobinMaesPrior reviewed Dec 11, 2024
View reviewed changes
Copy link
Collaborator

@RobinMaesPrior RobinMaesPrior left a comment

Choose a reason for hiding this comment

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

I think rooting_depth would need to be called in the nodes section too, not just in overrides

RobinMaesPrior
RobinMaesPrior reviewed Dec 11, 2024
View reviewed changes
Copy link
Collaborator

@RobinMaesPrior RobinMaesPrior left a comment •
edited
Loading

Choose a reason for hiding this comment

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

These changes would mean a GrowingSurface parameter is set, overrided and tested (ET_depletion_factor was already set)

tests/test_land.py Outdated Show resolved Hide resolved
tests/test_land.py Outdated Show resolved Hide resolved
barneydobson and others added 3 commits December 12, 2024 09:22
@barneydobson @RobinMaesPrior
Update tests/test_land.py
3fdd33d 
Co-authored-by: Robin Maes-Prior <87651497+RobinMaesPrior@users.noreply.github.com>
@barneydobson @RobinMaesPrior
Update tests/test_land.py
a87eeb1 
Co-authored-by: Robin Maes-Prior <87651497+RobinMaesPrior@users.noreply.github.com>
@pre-commit-ci
[pre-commit.ci] auto fixes from pre-commit.com hooks
5651d26 
for more information, see https://pre-commit.ci
@barneydobson barneydobson merged commit d05486d into main Dec 12, 2024
20 checks passed
@barneydobson barneydobson deleted the apply-surface-overrides branch December 12, 2024 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RobinMaesPrior RobinMaesPrior RobinMaesPrior approved these changes

@liuly12 liuly12 liuly12 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Overriding land node surface parameters
3 participants
@barneydobson @RobinMaesPrior @liuly12