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

Added TypeAlias #52452

Closed
wants to merge 2 commits into from
Closed

Added TypeAlias #52452

wants to merge 2 commits into from

Conversation

sjdex
Copy link

@sjdex sjdex commented Apr 5, 2023

@MarcoGorelli MarcoGorelli self-requested a review April 5, 2023 17:24
no_default: Final = _NoDefault.no_default
NoDefault = Literal[_NoDefault.no_default]
no_default: Final = ...
NoDefault = ...
Copy link
Member

Choose a reason for hiding this comment

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

These changes also caused errors for mypy&pyright: https://github.com/pandas-dev/pandas/actions/runs/4620671070/jobs/8171100725?pr=52452

If

no_default: Final[TypeAlias] = _NoDefault.no_default
NoDefault: TypeAlias = Literal[_NoDefault.no_default]

doesn't work, you can suppress those errors.

Copy link
Member

Choose a reason for hiding this comment

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

Might need to ignore these two lines, mypy reports many errors like these:

error: Expression of type "TypeAlias" cannot be assigned to parameter of type "bool | Literal[_NoDefault.no_default]"
Type "TypeAlias" cannot be assigned to type "bool | Literal[_NoDefault.no_default]"

Copy link
Member

@twoertwein twoertwein left a comment

Choose a reason for hiding this comment

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

typing.TypeAlias exists only since 3.10 - while this doesn't cause runtime issues, mypy complains about it. You can change the imports probably to from typing_extensions import TypeAlias

@sjdex
Copy link
Author

sjdex commented Apr 6, 2023

After running pre-commit run ruff --all-files , getting around 26 errors related to line formatting. See below
5C39E5C9-9647-45EC-B6CF-34A6962325EF

Do I need to fix this in this Issue itself, or is there going to be a seperate one.?

@MarcoGorelli
Copy link
Member

they'd need to be fixed as part of the same PR, CI need to be green before merging

what's causing the long lines now?

@sjdex
Copy link
Author

sjdex commented Apr 6, 2023

Not sure whats causing this, as lines are within limit of 88 chars. Breaking up the line solves this issue though.

@MarcoGorelli
Copy link
Member

if you push a commit I can take a look

@MarcoGorelli
Copy link
Member

I think this is a false-positive in ruff itself - I've reported it to them here: astral-sh/ruff#3902

let's put this on hold until they get back to us

@MarcoGorelli
Copy link
Member

I think this is a false-positive in ruff itself - I've reported it to them here: charliermarsh/ruff#3902

let's put this on hold until they get back to us

from astral-sh/ruff#3902 , it looks like we may just have to further split the lines

@mroeschke mroeschke added Code Style Code style, linting, code_checks Typing type annotations, mypy/pyright type checking labels Apr 6, 2023
njit = jit
generated_jit = jit
njit = ...
generated_jit = ...
Copy link
Member

Choose a reason for hiding this comment

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

I'm quite sure we want to keep those? can they be a TypeAlias?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure they can. It seems that TypeAlias is imported from the wrong library. It should be imported from typing_extensions.

@sjdex sjdex closed this Apr 26, 2023
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 Typing type annotations, mypy/pyright type checking
Projects
None yet
Development

Successfully merging this pull request may close these issues.

STYLE use TypeAlias and upgrade ruff
5 participants