-
Notifications
You must be signed in to change notification settings - Fork 14
Add constructors API #1367
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.
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
Add constructors API #1367
Conversation
13ed949
to
2c9f7b2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, Alexis! Im glad this is working.
We also have a number of model constructors which we should add. I referenced some here, but I am sure I missed some. I think we basically want any constructor for a parameter, or model, in the docs, especially the "convenience" ones that make it easy to setup models from forcing, domain, and the toml dict.
This includes integrated models too (land, soilcanopy, soil snow, soil biogeochemistry).
2c9f7b2
to
f7a8c5e
Compare
The integrated models have inner constructors, and the struct are in the API already, https://clima.github.io/ClimaLand.jl/stable/APIs/ClimaLand/ - I am not sure if we want to do something differently? |
There are also some outer constructors for the integrated models: ClimaLand.jl/src/integrated/land.jl Line 122 in 6e43b5c
ClimaLand.jl/src/integrated/land.jl Line 290 in 6e43b5c
(for the LandModel - similar for soilcanopy) |
f7a8c5e
to
86ba544
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is everything but the canopy model constructors:
function CanopyModel{FT}(; |
function CanopyModel{FT}( |
and the couple of comments left below. thank you!
An outer constructor for `SnowParameters` which supplies defaults for | ||
all arguments but `earth_param_set`. | ||
""" | ||
function SnowParameters{FT}( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this constructor is used in a few more places that need to be updated:
ClimaLand.jl/src/standalone/Snow/Snow.jl
Line 406 in 6e43b5c
return SnowParameters{FT}( ClimaLand.jl/src/standalone/Snow/Snow.jl
Line 492 in 6e43b5c
parameters = SnowParameters{FT}( - and a few tests
5c11661
to
bc9db15
Compare
closes #1346 Only the structs themselves show up in the API docs, not the constructor functions. This commit fixes this issue.
bc9db15
to
805e901
Compare
closes #1346
preview: https://clima.github.io/ClimaLand.jl/previews/PR1367/
Note: before merging, need to checkout docs/make.jl
Only the structs themselves show up in the API docs, not the constructor functions.
This commit fixes this issue.