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

Rename type aliases for consistency #162

Merged
merged 2 commits into from
Nov 21, 2024
Merged

Rename type aliases for consistency #162

merged 2 commits into from
Nov 21, 2024

Conversation

inducer
Copy link
Owner

@inducer inducer commented Nov 20, 2024

This was straightforward enough to make with pyright's symbol rename support. I'm not sure I fully understand the compatibility implications.

@inducer inducer requested a review from alexfikl November 20, 2024 22:07
@inducer inducer force-pushed the rename-type-aliases branch 5 times, most recently from ea1509d to a6049ac Compare November 21, 2024 00:36
@inducer inducer marked this pull request as ready for review November 21, 2024 00:39
@inducer
Copy link
Owner Author

inducer commented Nov 21, 2024

That's probably it in terms of compatibility impact. Other than a better name for ExpressionOrData, I'm OK with this.

Copy link
Collaborator

@alexfikl alexfikl left a comment

Choose a reason for hiding this comment

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

Looks good to me too!

Not sure if I can come up with a better than ExpressionOrData though.

  • ExpressionOrScalar? (is there other "data" you had in mind?)
  • NestedExpression? (because of the recursive typevar?)
  • Not sure it's a good idea to go very general here with something like Node or SymbolicNode, ExpressionNode?

@inducer inducer force-pushed the rename-type-aliases branch from a6049ac to cce6625 Compare November 21, 2024 15:37
@inducer inducer force-pushed the rename-type-aliases branch from c972660 to ce6dc1e Compare November 21, 2024 16:16
@inducer inducer enabled auto-merge (rebase) November 21, 2024 16:17
@inducer inducer disabled auto-merge November 21, 2024 16:24
@inducer inducer enabled auto-merge (rebase) November 21, 2024 16:25
@inducer inducer merged commit 31dbd39 into main Nov 21, 2024
11 checks passed
@inducer inducer deleted the rename-type-aliases branch November 21, 2024 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants