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.

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

feat: add soil moisture and freeze thaw init config models #96

Conversation

aaraney
Copy link
Member

@aaraney aaraney commented Feb 7, 2024

Additions

Notes

@aaraney aaraney force-pushed the add-soil-moisture-and-freeze-thaw-init-configs branch 3 times, most recently from 0db58c1 to 820b789 Compare February 22, 2024 15:14
@aaraney aaraney force-pushed the add-soil-moisture-and-freeze-thaw-init-configs branch from 820b789 to 4949d14 Compare March 8, 2024 18:08
@aaraney aaraney marked this pull request as ready for review March 8, 2024 18:08
@aaraney aaraney force-pushed the add-soil-moisture-and-freeze-thaw-init-configs branch 6 times, most recently from a0e9735 to a10b1f9 Compare March 9, 2024 17:09
@hellkite500 hellkite500 self-assigned this Mar 15, 2024
@hellkite500
Copy link
Member

hellkite500 commented Jun 27, 2024

Were these TODO's ever addressed?

satpsi defaults to 0.3
check initial_psi lower bound

And can you rebase the conflict?

Active development on lgar BMI may require some iterative changes to the lgar init config, but I think we can put these in place and track updates/issues as they are used

@aaraney aaraney force-pushed the add-soil-moisture-and-freeze-thaw-init-configs branch from a10b1f9 to edc67cc Compare July 2, 2024 14:35
@aaraney
Copy link
Member Author

aaraney commented Jul 2, 2024

Were these TODO's ever addressed?

satpsi defaults to 0.3
check initial_psi lower bound

Nope and looking back I don't think there was enough information in the source code of the bmi modules for me to gather that information. So, the TODOs were more long term, "hey, im thinking about this long term." IMO, I don't think they need to be addressed in this PR.

@aaraney
Copy link
Member Author

aaraney commented Jul 2, 2024

@hellkite500, just rebased and looked through things again. This should be good to merge now!

Wrapper around a List[V] that coerces a comma separated string of Vs
(e.g. "1,2,3") into a List[V] (e.g. [1, 2, 3]). The `dict` method is
overridden to return the inner list of Vs instead of {"__root__": [Vs...]}.
As a result, using a CSList[V] as a field type in anther pydantic model
behaves as if it were a List[V] with the added functionality of coercing a
comma separated string into a list of Vs.

The features this model provides could easily be achieved using a field
level validator, so in some ways this is provided as a convenience. This
model is most useful when a field can be multiple types (union type). It
alleviates the need to implement a complex field level validator that
handles all possible union type members.
@aaraney aaraney force-pushed the add-soil-moisture-and-freeze-thaw-init-configs branch from edc67cc to 03779d2 Compare August 22, 2024 20:04
time_unit = Literal["s", "sec", "", "h", "hr", "d", "day"]


class IceFractionScheme(str, Enum):
Copy link
Member

Choose a reason for hiding this comment

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

I thought these were rainfall runoff partitioning schemes, not Ice Fraction schemes 🤯

@hellkite500 hellkite500 merged commit 6ff6e61 into NOAA-OWP:master Aug 22, 2024
13 checks passed
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.

2 participants