-
Notifications
You must be signed in to change notification settings - Fork 24
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
Cleanup PR #2 (v2) #156
Cleanup PR #2 (v2) #156
Conversation
The following tools do run once
Additionally, these scripts can now be run from an
A few outstanding items to address:
|
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 looks great! One small fix, but otherwise it's ready to go
@@ -111,7 +111,7 @@ compute_notebooks: | |||
|
|||
atm: | |||
link_to_ADF: | |||
kernel_name: cupid-dev | |||
kernel_name: cupid-analysis |
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.
link_to_ADF
used cupid-dev
because it needs to import IPython
to inline plots and create links... so this should be cupid-infrastructure
rather than cupid-analysis
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 for noting this! I just made that change.
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:
This PR is intended to close #152 by splitting
run
intotimeseries
anddiagnostics
, renamingbuild
towebpage
, and makingtimeseries
,diagnostics
, andwebpage
scripts executable.pre-commit
checks passed (#8 in Adding Notebooks Guide)?