-
Notifications
You must be signed in to change notification settings - Fork 12
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
Add analysis tools #273
Add analysis tools #273
Conversation
…n the style of the analysis-scripts
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.
Well aware this is still in draft mode, but this is not far off from being able to be merged at all. Excellent work!
This needs the new noaa-gfdl::analysis_scripts
package req added to environment.yml
. If that module is indended to be pure-python package, we should add it to setup.py
as well.
Without this, a currently doc'd development approach involving conda env create -f environment.yml
breaks, and thats why create_test_conda_env.yaml
is likely failing.
After doing that and making sure the pipeline passes, merge the latest 100 or commits to main
into this branch to make sure nothing clashes. I doubt any clashes will come up... but no need to speculate, we have a pipeline!
Thrilled to approve this when its ready
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 might be a lesson in why one never says, "it's almost done!" Apologies for bringing a pox-upon-the-house!
I'm pretty sure this is getting caught up on some of click
and python
's most classic pit falls. I think I caught most of them.
…nto add-analysis-tools
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.
We note a few pylint
nits that have no impact, and some venv
error handling within analysis_scripts
that could be improved with some thought.
I think we're ready to merge this and to start exploring what's possible with the new functionality. This is a great step in fre-cli
's development. Thanks @menzel-gfdl and @ceblanton for the quality work!
Describe your changes
Add some analysis tools to:
Issue ticket number and link (if applicable)
Checklist before requesting a review