8000 Third-party stubs: enforce CamelCase for type alias names by AlexWaygood · Pull Request #8256 · python/typeshed · GitHub
[go: up one dir, main page]
Skip to content

Third-party stubs: enforce CamelCase for type alias names #8256

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Jul 19, 2022
Merged

Third-party stubs: enforce CamelCase for type alias names #8256

merged 8 commits into from
Jul 19, 2022

Conversation

AlexWaygood
Copy link
Member
@AlexWaygood AlexWaygood commented Jul 7, 2022 •
edited
Loading

@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
AlexWaygood
AlexWaygood commented Jul 7, 2022
View reviewed changes
@github-actions

This comment has been minimized.

Sign in to view
1 similar comment
@github-actions

This comment has been minimized.

Sign in to view
@srittau
Copy link
Collaborator
srittau commented Jul 7, 2022

A lot of aliases are marked as type aliases that I wouldn't consider type aliases. In my opinion as soon as an alias is used at runtime, I wouldn't consider it a type aliases. Examples are ordered_column_set in SQLAlchemy or pmem in psutil.

@AlexWaygood
Copy link
Member Author
AlexWaygood commented Jul 7, 2022 •
edited
Loading

A lot of aliases are marked as type aliases that I wouldn't consider type aliases. In my opinion as soon as an alias is used at runtime, I wouldn't consider it a type aliases. Examples are ordered_column_set in SQLAlchemy or pmem in psutil.

I agree that these don't need to be type aliases necessarily. @hauntsaninja raised some objections the last time I tried to remove those markers from some of them (#8248 (review)), but I also don't think he has a strong objection (#8248 (review))

@github-actions

This comment has been minimized.

Sign in to view
@github-actions

This comment has been minimized.

Sign in to view
@AlexWaygood
Copy link
Member Author

@srittau, does 27308f3 address your concerns?

@github-actions

This comment has been minimized.

Sign in to view
JelleZijlstra
JelleZijlstra reviewed Jul 19, 2022
View reviewed changes
stubs/SQLAlchemy/sqlalchemy/util/_collections.pyi
# Ignore Y026, this isn't a type alias.
# We have to do `ordered_column_set = OrderedSet[_T]
# instead of `ordered_column_set = OrderedSet`,
# or pyright complains
Copy link
Member

Choose a reason for hiding this comment

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

Why doesn't pyright complain about column_set?

Copy link
Member Author
@AlexWaygood AlexWaygood Jul 19, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

I have no idea :/

JelleZijlstra
JelleZijlstra approved these changes Jul 19, 2022
View reviewed changes
Copy link
Member
@JelleZijlstra JelleZijlstra left a comment

Choose a reason for hiding this comment

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

One optional suggestion

@AlexWaygood @JelleZijlstra
Update stubs/psycopg2/psycopg2/_psycopg.pyi
53c160c 
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

@AlexWaygood AlexWaygood merged commit e3d4bdc into master Jul 19, 2022
@AlexWaygood AlexWaygood deleted the third-party-aliases branch July 19, 2022 07:23
@AlexWaygood AlexWaygood mentioned this pull request Nov 11, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JelleZijlstra JelleZijlstra JelleZijlstra approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@AlexWaygood @srittau @JelleZijlstra
0