Skip to content

Conversation

@xylar
Copy link
Contributor

@xylar xylar commented Oct 10, 2025

Summary

Some dependencies were updated recently in #25 and #38 in ways I do not think we want in the end. This PR aims to fix those dependencies

Objectives:

  • Put back constraints that I do not think should have been dropped because they were put on the tools for a reason
  • Get tools from conda-forge whenever possible

Select one: This pull request is...

  • a bug fix: increment the patch version
  • a small improvement: increment the minor version
  • a new feature: increment the minor version
  • an incompatible (non-backwards compatible) API change: increment the major version

Please fill out either the "Small Change" or "Big Change" section (the latter includes the numbered subsections), and delete the other.

Small Change

  • To merge, I will use "Squash and merge". That is, this change should be a single commit.
  • Logic: I have visually inspected the entire pull request myself.
  • Pre-commit checks: All the pre-commits checks have passed.

- flake8 ==7.3.0
- isort ==6.0.1
#- mypy ==1.18.2
- mypy ==1.18.2
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If, in the future, the latest version of mypy or another pinned linting or CI tool is not available on conda-forge, use the latest version available on conda-forge both here and in the pre-commit config file, rather than pulling the tool from PyPI. This was @tomvothecoder's suggestion the last time this happened as well.

This was referenced Oct 10, 2025
Copy link
Collaborator

@forsyth2 forsyth2 left a comment

Choose a reason for hiding this comment

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

Does the pyproject.toml also need the following?

-    "matplotlib",
+    "matplotlib-base >=3.8.2",

I also noticed in that file:

qa = [
    "black==24.10.1",
    "flake8==7.1.1",
    "flake8-isort==6.1.0",
    "isort==5.13.2",
    "mypy==1.11.2",
    "pre-commit >=3.0.0",
    "types-PyYAML >=6.0.0",
]

Should these match the versions in .pre-commit-config.yaml?

@xylar
Copy link
Contributor Author

xylar commented Oct 10, 2025

matplotlib-base is a simplified package only available on conda-forge. It is the preferred dependency if you don't need the features of the full maptplotlib (which, in practice, we never do). So we want matplotlib >=3.8.2 in the pyproject.toml. But PyPI doesn't know about maptplotlib-base so we won't include that there.

Yes, I believe the qa section of the pyproject.toml should match both the pre-commit config and the dev.yml. Please push fixes to both of those to this PR if you like. I will add you as a collaborator.

@xylar
Copy link
Contributor Author

xylar commented Oct 10, 2025

@forsyth2, actually, I'll just take care of it. Just as sec...

Comment on lines +46 to 52
"black==25.1.0",
"flake8==7.3.0",
"flake8-isort==6.1.1",
"isort==6.0.1",
"mypy==1.18.2",
"pre-commit==4.3.0",
"types-PyYAML >=6.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@forsyth2, please double check these.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes, these appear to match the current dev.yml and .pre-commit-config.yml.

@forsyth2
Copy link
Collaborator

I'll just take care of it.

Thanks!!

@xylar
Copy link
Contributor Author

xylar commented Oct 10, 2025

@forsyth2, I think this is ready for you to merge if you're ready.

@forsyth2
Copy link
Collaborator

GitHub Actions are passing and visual inspection looks good, merging. Thanks again @xylar!

@forsyth2 forsyth2 merged commit 96e331b into E3SM-Project:main Oct 10, 2025
5 checks passed
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.

2 participants