Skip to content

STYLE: Use from future import annotations (pandas-dev#41901) #41941

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

Conversation

ForestMars
Copy link

@ForestMars ForestMars commented Jun 11, 2021

Implementation:

  • Name set to always-use-future-annotations (rather than always-use-from-future-annotations)
  • We filter with types: [python] rather than files: "\\.(py)$”
  • No minimum pre-commit version.

What else is needed here for a merge?

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

Thanks @ForestMars

Did you try running pre-commit run always-use-future-annotations --all-files? That would need to pass

You probably also want to exclude __init__.py files, setup.py, api.py, and tests

@simonjayhawkins simonjayhawkins added Code Style Code style, linting, code_checks Typing type annotations, mypy/pyright type checking labels Jun 11, 2021
@ForestMars
Copy link
Author

ForestMars commented Jun 11, 2021

edited comment to note that @aacosta13 has a script to add "from future import annotations" to the 120 files in the codebase that currently don't have it (excluding tests, init.py, etc.)

I suggested that should be a separate PR which can go in first.

@MarcoGorelli
Copy link
Member

@ForestMars yeah - can exclude asv_bench and web too though

@ForestMars ForestMars requested a review from MarcoGorelli June 11, 2021 21:10
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

The check's still failing - can you add the import to those files, or exclude them from the check? doc, ci, and versioneer.py can definitely be excluded

@ForestMars
Copy link
Author

by way of update, I was waiting on a PR from @aacosta13 so that all targeted (ie. not excluded) files have the required import added, before we merge the pre-commit hook into the code base. I originally thought this was something that was going to be a quick fix, but haven't seen an update on #41901 in over a week.

@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label Jul 29, 2021
@MarcoGorelli
Copy link
Member

by way of update, I was waiting on a PR from @aacosta13 so that all targeted (ie. not excluded) files have the required import added, before we merge the pre-commit hook into the code base. I originally thought this was something that was going to be a quick fix, but haven't seen an update on #41901 in over a week.

feel free to take over the other PR if you're still interested in working on this

@mroeschke
Copy link
Member

Thanks for the PR. As mentioned, it appears a precursor PR is needed first (which we're a happy to have from you) first. Since this PR has gone stale in the meantime, going to close for now.

@mroeschke mroeschke closed this Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Code Style Code style, linting, code_checks Stale Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

STYLE add check for future annotations
4 participants