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

Make pattern classes dataclasses #1346

Merged
merged 1 commit into from
Sep 6, 2024
Merged

Conversation

plypaul
Copy link
Contributor

@plypaul plypaul commented Jul 19, 2024

This PR makes pattern classes dataclass for easier comparison for equality. The comparison is helpful when memoizing / caching function calls.

Copy link

Thank you for your pull request! We could not find a changelog entry for this change. For details on how to document a change, see the contributing guide.

@plypaul plypaul marked this pull request as ready for review July 19, 2024 19:29
Copy link
Contributor

@tlento tlento left a comment

Choose a reason for hiding this comment

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

The .create pattern is odd, probably worth cleaning up before merge.

Comment on lines +22 to +23
def create(listed_specs: Sequence[InstanceSpec]) -> MatchListSpecPattern: # noqa: D102
return MatchListSpecPattern(tuple(listed_specs))
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this necessary? Just to avoid the callsite typecasting? Might be nice to have a docstring indicating that, but really, since there's exactly one callsite, why not just wrap the argument in that one call in tuple() instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was in the habit of doing this for some other changes in the stack, but now that you mention it, this one does not need it. Updated.

@plypaul plypaul force-pushed the p--query-resolution-perf--16 branch from 8c2baac to 7bf03b5 Compare September 6, 2024 17:21
@plypaul plypaul changed the base branch from p--query-resolution-perf--15 to main September 6, 2024 17:22
@plypaul plypaul merged commit 27218b2 into main Sep 6, 2024
14 checks passed
@plypaul plypaul deleted the p--query-resolution-perf--16 branch September 6, 2024 17:41
@plypaul
Copy link
Contributor Author

plypaul commented Sep 6, 2024

And I forgot to push those updates.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants