Skip to content

STYLE: enable ruff TCH on some file #51835

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

Merged
merged 8 commits into from
Mar 9, 2023

Conversation

luke396
Copy link
Contributor

@luke396 luke396 commented Mar 8, 2023

xref #51740, to fix

"pandas/core/construction.py" = ["TCH"] 
"pandas/core/missing.py" = ["TCH"] 
"pandas/tseries/*" = ["TCH"] 
"pandas/tests/*" = ["TCH"] 
"pandas/plotting/*" = ["TCH"] 

@@ -1,3 +1,4 @@
# ruff: noqa: TCH004
Copy link
Contributor Author

Choose a reason for hiding this comment

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

same as #51812

Comment on lines 13 to 14
from pandas._typing import Dtype # noqa: TCH001
from pandas._typing import PositionalIndexer # noqa: TCH001
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 add TYPE_CHECKING, will cause import error, in pandas/tests/io/json/test_json_table_schema_ext_dtype.py

Copy link
Member

Choose a reason for hiding this comment

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

any idea why? would it work to add from __future__ import annotations at the top of this file?

Copy link
Member

Choose a reason for hiding this comment

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

Based on the pytest failures, adding a future import to pandas/tests/extension/date/array.py should fix this. (Wasn't there a script to enforce that all files have future imports?)

Copy link
Contributor Author

@luke396 luke396 Mar 9, 2023

Choose a reason for hiding this comment

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

It seems like there have been a lot of changes to the type annotations, and I can't determine whether they will have a significant impact or not.

Edit: if we add a script to enforce all files have future imports, the type annotation maybe change a lot too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It seems like there have been a lot of changes to the type annotations, and I can't determine whether they will have a significant impact or not.

Edit: if we add a script to enforce all files have future imports, the type annotation maybe change a lot too.

I searched for some information and found that these type annotation changes might be due to a new annotation feature introduced in a newer version of Python. It seems that this change does not have a significant impact, except for compatibility with older versions of Python.

Copy link
Member

Choose a reason for hiding this comment

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

the check excludes test files (though perhaps it should include them?)

- id: future-annotations
name: import annotations from __future__
entry: 'from __future__ import annotations'
language: pygrep
args: [--negate]
files: ^pandas/
types: [python]
exclude: |
(?x)
/(__init__\.py)|(api\.py)|(_version\.py)|(testing\.py)|(conftest\.py)$
|/tests/
|/_testing/

Copy link
Contributor Author

@luke396 luke396 Mar 11, 2023

Choose a reason for hiding this comment

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

the check excludes test files (though perhaps it should include them?)

- id: future-annotations
name: import annotations from __future__
entry: 'from __future__ import annotations'
language: pygrep
args: [--negate]
files: ^pandas/
types: [python]
exclude: |
(?x)
/(__init__\.py)|(api\.py)|(_version\.py)|(testing\.py)|(conftest\.py)$
|/tests/
|/_testing/

Sorry for the late reply. If necessary, I can open a pr and manually or automatically add 'from future import annotations'.

@MarcoGorelli
Copy link
Member

thanks for your pr, looks like there's some errors https://github.com/pandas-dev/pandas/actions/runs/4362414904/jobs/7627283459

@luke396
Copy link
Contributor Author

luke396 commented Mar 8, 2023

thanks for your pr, looks like there's some errors https://github.com/pandas-dev/pandas/actions/runs/4362414904/jobs/7627283459

This was because of a careless mistake and should be fine now

@mroeschke mroeschke added the Code Style Code style, linting, code_checks label Mar 8, 2023
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 @luke396 ! not many left, we're getting there

@MarcoGorelli MarcoGorelli added this to the 2.1 milestone Mar 9, 2023
@MarcoGorelli MarcoGorelli merged commit 7ffb960 into pandas-dev:main Mar 9, 2023
@luke396 luke396 deleted the enable-ruff-TCH-on-some-files branch March 11, 2023 00:09
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants