-
Notifications
You must be signed in to change notification settings - Fork 6
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
Remove alias
field and AliasParamDict
functionality, replace with name mapping
#154
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #154 +/- ##
==========================================
+ Coverage 94.73% 98.46% +3.72%
==========================================
Files 1 1
Lines 171 130 -41
==========================================
- Hits 162 128 -34
+ Misses 9 2 -7 ☔ View full report in Codecov by Sentry. |
a90753c
to
725361e
Compare
alias
field and AliasParamDict
functionality, replace with ParameterNameMap
alias
field and AliasParamDict
functionality, replace with name mapping
21b9566
to
6fee7f5
Compare
6fee7f5
to
9a8b360
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.
Looks great! Please merge after addressing the docs comments - I'm glad we finally got to removing the aliases.
e360042
to
4a7443f
Compare
Purpose
This is part of the larger SDI #144 . The
alias
field in the TOML dictionary is overly restrictive, but the core functionality of mapping long parameter names to code variable names needs to be maintained.Content
This PR introduces name maps, which are essentially a dictionary that maps from long parameter names found in the TOML file to short variable names found in code.
This is mainly used in
get_parameter_values
, which can now take a name map instead of a list of parameter names.get_parameter_values
uses the name mapping to find the correct parameters and returns their short names and values in a namedtuple.Example:
A name map does not strictly need to be a Dict. It can be a Vector, Tuple, or Varargs of Pairs.
This replaces the short name
alias
functionality and makes naming conventions local to specific repositories/modules. The downside here is that each package will have to declare their own name map if they want to use short names for parameter variables effectively.Here is how I see it being used:
Additional Changes
create_parameter_struct(struct_type, toml_dict; name_map)
is a generalized constructor for parameter structs. This already exists in ClimaAtmos, but needs to be moved to ClimaParameters and made a bit less restrictive.Tests for name maps
alias
field removed from the default TOMLRename
docs/src/toml_dict.md
todocs/src/param_retrieval.md
and add more information. The docs will be improved in a future PR.Removes
get_parameter_values!
- logging can still be done viaget_parameter_values