Add shr_string_listGetAllNames and associated unit tests#75
Add shr_string_listGetAllNames and associated unit tests#75billsacks merged 6 commits intoESCOMP:mainfrom
Conversation
nusbaume
left a comment
There was a problem hiding this comment.
Thanks for the new functionality @billsacks! I just had some (hopefully minor) requests/questions.
Co-authored-by: Jesse Nusbaumer <nusbaume@ucar.edu>
Co-authored-by: Jesse Nusbaumer <nusbaume@ucar.edu>
|
Thanks a lot for your review, @nusbaume ! I accepted your suggested changes – thank you for those. I have responded to your other comments, with my explanations for why I propose not changing things; I'm very open to discussing those further if you'd like. (The pFUnit expected-error handling is something that I've had extensive discussions about both with CTSM developers - especially Adrianna - and with Tom Clune (the original developer of pFUnit)... I'd be happy to talk more with you about that if you want.) |
nusbaume
left a comment
There was a problem hiding this comment.
Thanks for the revisions and discussion @billsacks! I found one last location that could use list markers, but otherwise everything looks great to me. Thanks again!
Co-authored-by: Jesse Nusbaumer <nusbaume@ucar.edu>
I plan to leverage this new subroutine in the water tracer code: From the config file, we'll read in colon-delimited lists of water tracer names, water tracer species, and water tracer initial ratios; we then will need to turn each of those into arrays of values; this new subroutine accomplishes that step.
I also added some unit tests of shr_string_listIsValid. I was originally planning to modify that routine to add some flexibility in terms of what's considered "valid", but that started to feel messy, so I'm instead planning to keep that as is. But before deciding to leave it as is, I had already written some unit tests to cover my planned rework; I figured I might as well leave them in place to cover this code for the future. If anyone is interested (probably mainly @nusbaume ): The two changes I had been thinking of making to this were (1) optionally allowing 0-length lists (in case we have 0 water tracers), and (2) optionally allowing 0-length elements (in case the species name is empty, indicating bulk water). For (1), I decided to handle this by checking this in the caller and doing special handling of 0-length lists. For (2), I decided to use "-" as the species name for bulk water rather than an empty string so that I could use the shr_string_list functionality unchanged.