Skip to content
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

Adding reduced ascii file + updating pytest to convert it to binary #215

Merged
merged 21 commits into from
Dec 19, 2024

Conversation

Ciheim
Copy link
Collaborator

@Ciheim Ciheim commented Oct 22, 2024

Describe your changes

Issue ticket number and link (if applicable)

Checklist before requesting a review

  • I ran my code
  • I tried to make my code readable
  • I tried to comment my code
  • I wrote a new test, if applicable
  • I wrote new instructions/documentation, if applicable
  • I ran pytest and inspected it's output
  • I ran pylint and attempted to implement some of it's feedback

Copy link
Collaborator

@ceblanton ceblanton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! I didn't give it a try yet, but doesn't the .cdl file need some data points on the reduced grid?

@ilaflott
Copy link
Member

Looking good- can you git rm the netCDF file that reduced_ocean_monthly_1x1deg.199301-199712.sos.cdl + ncgen replaces? That way, anywhere there's a pytest failure, you'll know there's fixing to do

@ilaflott ilaflott added the new functioning New feature or request label Oct 28, 2024
@ceblanton
Copy link
Collaborator

Is this ready? The tests may not be passing, but maybe it just needs a re-try.

@ceblanton
Copy link
Collaborator

  1. Update the .cdl file to include the reduced grid input data
  2. pull in updates from main, and add recent tests
  3. set to undraft when ready for review

Copy link
Collaborator Author

@Ciheim Ciheim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Made needed changes

@Ciheim Ciheim marked this pull request as ready for review November 7, 2024 18:38
@ilaflott
Copy link
Member

ilaflott commented Nov 7, 2024

Made needed changes

the checks fail in create_test_conda_env but not build_conda 😕

@ceblanton
Copy link
Collaborator

@ilaflott @Ciheim we're discussing this and still want this!

@ilaflott ilaflott dismissed ceblanton’s stale review December 19, 2024 20:57

i'm certain @ceblanton would approve this. We're all running around like chickens with no heads at the moment!!! the code follows the current pattern. the test works. even generalizing to other tests should be clear enough!

@ilaflott ilaflott merged commit ba5f19b into main Dec 19, 2024
2 checks passed
@ilaflott ilaflott deleted the CMORPytestUpdates branch December 19, 2024 20:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new functioning New feature or request priority: HIGH
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants