-
Notifications
You must be signed in to change notification settings - Fork 26
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
Include global level obs repo directory #191
Conversation
@@ -57,20 +57,21 @@ | |||
}, | |||
"outputs": [], | |||
"source": [ | |||
"CESM_output_dir = \"/glade/campaign/cesm/development/cross-wg/diagnostic_framework/CESM_output_for_testing\"\n", |
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.
Hi @dabail10 , would it be alright with you if we remove these hard coded parameters from the cell, and instead make it clear that they come from the config file by just providing empty strings? @mnlevy1981 said that you were perhaps wanting to run this notebook outside of CUPiD as well, but I'm thinking you probably won't need these particular defaults anyways, and they may add more confusion.
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.
Dave and I discussed, and I'll comment these out to avoid confusion but leave them as guidance in case someone is running this notebook outisde of CUPiD.
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.
LGTM!
Description of changes:
It would be beneficial to only specify the global obs repo once rather than in every description of particular components' notebook parameters. Then, each component config file section can specify the latter part of the path to their observations.
All PRs Checklist:
pre-commit
checks passed (#8 in Adding Notebooks Guide)?