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

Add typechecking #73

Merged
merged 2 commits into from
Jun 6, 2023
Merged

Add typechecking #73

merged 2 commits into from
Jun 6, 2023

Conversation

mfisher87
Copy link
Collaborator

@mfisher87 mfisher87 commented Jun 5, 2023

Depends on #72 -- after merging, this branch should be rebased / merged from main

This PR adds type checking for annotations added in #72

# variable`, unless this variable is set:
MPLBACKEND: "Agg"

- name: "Install mypy"
Copy link
Collaborator Author

@mfisher87 mfisher87 Jun 5, 2023

Choose a reason for hiding this comment

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

Added to this workflow because we already have the project dependencies installed here. But we don't have a reason to run mypy on 4 different Python versions (it's configured to target 3.8 in pyproject.toml). Creating a Python environment is expensive, but so is running the type checker. I don't have strong feelings about this tradeoff 🤷 Happy to make this a separate workflow.

@mfisher87 mfisher87 marked this pull request as ready for review June 5, 2023 02:19
@mfisher87 mfisher87 requested a review from stefanv June 5, 2023 02:19
Copy link
Contributor

@stefanv stefanv left a comment

Choose a reason for hiding this comment

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

Looks fine, although I don't think we have type annotations so not sure we'll learn much from mypy.

@mfisher87
Copy link
Collaborator Author

I added some annotations in the previous PR :) I find a typechecker to be a really useful tool for refactoring as well.

https://github.com/matplotlib/viscm/pull/72/files#diff-bde4a0591061d9351a524a93d1ee9857407b24c71f2e317ba30a646795d85d93R117-R126

@mfisher87 mfisher87 force-pushed the add-typechecking branch 2 times, most recently from ac5bc73 to e02a37d Compare June 6, 2023 03:04
@stefanv stefanv merged commit 6735c7e into matplotlib:main Jun 6, 2023
5 checks passed
@mfisher87 mfisher87 deleted the add-typechecking branch June 6, 2023 18:45
@mfisher87 mfisher87 added this to the v0.10 milestone Jun 11, 2023
@mfisher87 mfisher87 added the ci label Jun 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants