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

Updated PR for seaice notebook. #138

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Updated PR for seaice notebook. #138

wants to merge 13 commits into from

Conversation

dabail10
Copy link
Collaborator

@dabail10 dabail10 commented Sep 27, 2024

All Submissions:

  • Have you followed the guidelines in our Contributor's Guide (including the pre-commit check)?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Feature Submissions:

  1. Does your submission pass tests?
  2. Have you lint your code locally prior to submission?

Changes to Core Features:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you successfully tested your changes locally?

I have made several updates to the sea ice notebook here.

  1. Modified the cice_vars.yml file to have only three variables. I have created a new cice_vars_full.yml file for other use cases.
  2. Changed the color tables from tab20 to ocean.
  3. Added some Antarctic plots.
  4. Added (in comments) a PBS DASK cluster option for running the notebook within jupyterhub.
  5. Modified the cases in config.yml to be historical runs for seaice.
  6. As an aside, I have also moved the timeseries files to ice/proc/tseries/month_1 for all of the cases instead of "ts".

@dabail10
Copy link
Collaborator Author

I apologize for all of the commit / push nonsense. I was trying to figure out the pre-commit stuff. I finally fixed it all!

@dabail10
Copy link
Collaborator Author

I guess I still need some help here with the PR Tasks.

@mnlevy1981
Copy link
Collaborator

Might be a little late now, but you can have pre-commit run these checks locally -- instructions are in the Contributor's Guide

@dabail10
Copy link
Collaborator Author

Might be a little late now, but you can have pre-commit run these checks locally -- instructions are in the Contributor's Guide

I tried to install and run pre-commit, but I couldn't figure it out. Is it just pip install pre-commit? I did this and it still did not find the command.

@mnlevy1981
Copy link
Collaborator

I guess I still need some help here with the PR Tasks.

That's meant to be a "pass if all the checklist items are complete, fail if something still needs to be done" but it isn't working quite right -- @TeaganKing is talking to the NCAR organization admin team to see about installing a better tool for that check. We can merge this in even if that check is failing, the pre-commit test is the important one

@mnlevy1981
Copy link
Collaborator

Might be a little late now, but you can have pre-commit run these checks locally -- instructions are in the Contributor's Guide

I tried to install and run pre-commit, but I couldn't figure it out. Is it just pip install pre-commit? I did this and it still did not find the command.

It should already be installed in (cupid-dev), so try activating that environment and then running the pre-commit commands

@dabail10
Copy link
Collaborator Author

Might be a little late now, but you can have pre-commit run these checks locally -- instructions are in the Contributor's Guide

I tried to install and run pre-commit, but I couldn't figure it out. Is it just pip install pre-commit? I did this and it still did not find the command.

It should already be installed in (cupid-dev), so try activating that environment and then running the pre-commit commands

Ok. So, I had not updated the cupid-dev environment in many months. We should probably have something in the workflow about this if we don't already.

@TeaganKing
Copy link
Collaborator

I guess I still need some help here with the PR Tasks.

That's meant to be a "pass if all the checklist items are complete, fail if something still needs to be done" but it isn't working quite right -- @TeaganKing is talking to the NCAR organization admin team to see about installing a better tool for that check. We can merge this in even if that check is failing, the pre-commit test is the important one

See #139 -- this should be implemented shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants