Skip to content

Conversation

@altheaden
Copy link
Contributor

Description

I noticed that pre-commit hasn't been updated in a while, so in this PR I used pre-commit autoupdate to update pre-commit's dependencies. I've also updated the pre-commit version (as well as its dependencies' versions) in conda-env/dev.yml. Additionally, in more recent versions of ruff-pre-commit, the previous hook ID (ruff) is now referred to as a "legacy" alias, with ruff-check being the preferred ID now, so I've updated that as well.

The only other place I noticed the versions being specifically stated in this repo are in auxiliary_tools/debug/985-perf-degrade - I'm not sure if you'd want the versions updated there as well, but I can add that to this PR if that's desired. Just let me know!

As an aside, I have created a weekly CI workflow which automatically creates a PR with any updates to pre-commit and its dependencies, and we currently use it on several of @xylar's repos (such as mache and polaris). If manually updating pre-commit is something that is annoying to keep track of, then that workflow might be useful for you guys! However, none of those other repos have the dependencies' versions listed in a separate file (the way they are in conda-env/dev.yml) so it would need to be tweaked to include that (or it could be a manual change somebody adds when approving the PR). I'm happy to help set it up if that sounds like a desired change.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

If applicable:

  • New and existing unit tests pass with my changes (locally and CI/CD build)
  • I have added tests that prove my fix is effective or that my feature works
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have noted that this is a breaking change for a major release (fix or feature that would cause existing functionality to not work as expected)

@xylar xylar requested a review from tomvothecoder July 3, 2025 20:28
@xylar xylar requested a review from chengzhuzhang July 3, 2025 20:29
@xylar xylar added the DevOps CI/CD, configuration, etc. label Jul 3, 2025
@altheaden
Copy link
Contributor Author

It looks like the newest mypy version is finding some additional issues, which is not surprising. I can try to help with that issue if you think it would be something I can fix, or downgrade the mypy version, or something else if needed.

@xylar xylar self-requested a review July 3, 2025 20:32
@xylar
Copy link
Contributor

xylar commented Jul 3, 2025

@altheaden, this looks good to me. I'm also happy to help with the mypy issues if that's desirable. @tomvothecoder and @chengzhuzhang, just let us know.

@tomvothecoder
Copy link
Collaborator

This is awesome. Thanks a lot @altheaden!

The only other place I noticed the versions being specifically stated in this repo are in auxiliary_tools/debug/985-perf-degrade - I'm not sure if you'd want the versions updated there as well, but I can add that to this PR if that's desired. Just let me know!

You can safely ignore the dependency files under auxiliary_tools/debug/985-perf-degrade. They were only used for investigating differences between environments.

As an aside, I have created a weekly CI workflow which automatically creates a PR with any updates to pre-commit and its dependencies, and we currently use it on several of @xylar's repos (such as mache and polaris). If manually updating pre-commit is something that is annoying to keep track of, then that workflow might be useful for you guys! However, none of those other repos have the dependencies' versions listed in a separate file (the way they are in conda-env/dev.yml) so it would need to be tweaked to include that (or it could be a manual change somebody adds when approving the PR). I'm happy to help set it up if that sounds like a desired change.

I've always wanted a way to automate updating the QA dependencies. I would love to have this implemented in e3sm_diags! Other repos including https://github.com/E3SM-Project/e3sm_to_cmip, https://github.com/E3SM-Project/zppy, https://github.com/E3SM-Project/zstash, and https://github.com/xCDAT/xcdat all follow the same structure. We can easily copy this workflow to those repos as well.

I'm also happy to help with the mypy issues if that's desirable.

@xylar sounds good if you have time. Otherwise I can help here if I happen to get to it before @altheaden.

@tomvothecoder
Copy link
Collaborator

FYI: I am addressing some of the mypy issues in #921. Specifically, the unused #type: ignore ones. This PR will be merged soon and your branch should be rebased on top of it.

@xylar
Copy link
Contributor

xylar commented Jul 10, 2025

@altheaden, I think you can rebase this one now.

@altheaden
Copy link
Contributor Author

Oops, I forgot to push my rebased branch, sorry! I will push it now.

@altheaden altheaden force-pushed the update-pre-commit branch from e16519e to 55f4ec5 Compare July 10, 2025 21:12
@chengzhuzhang chengzhuzhang mentioned this pull request Jul 11, 2025
9 tasks
@xylar
Copy link
Contributor

xylar commented Jul 12, 2025

@tomvothecoder, @altheaden, it looks like the new mypy is still unhappy even after the rebase.

@tomvothecoder
Copy link
Collaborator

@tomvothecoder, @altheaden, it looks like the new mypy is still unhappy even after the rebase.

I expected there to be some remaining mypy issues after the rebase. I will push a commit to fix them shortly.

- Pin `pre-commit >=4.2.0`
@tomvothecoder tomvothecoder merged commit 6a9202e into E3SM-Project:main Jul 15, 2025
6 checks passed
@altheaden altheaden deleted the update-pre-commit branch July 17, 2025 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

DevOps CI/CD, configuration, etc.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants