Skip to content

Conversation

ioistired
Copy link

@ioistired ioistired commented Oct 2, 2019

currently it's a table, which is more efficient than an if-else chain,
but less efficient. We can have the best of both worlds by converting https://bugs.python.org/issue32929#msg312829
into a loop that generates the table that we currently have. This should make the table easier to maintain and understand.

https://bugs.python.org/issue38366

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA).

Recognized GitHub username

We couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames:

@iomintz

This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for the contribution, we look forward to reviewing it!

@ioistired ioistired changed the title bpo-NNNN: dataclasses: generate the _hash_action table more cleanly bpo-38366: dataclasses: generate the _hash_action table more cleanly Oct 3, 2019
currently it's a table, which is more efficient than an if-else chain,
but less efficient. We can have the best of both worlds by converting https://bugs.python.org/issue32929#msg312829
into a loop that generates the table that we currently have.
@ioistired ioistired force-pushed the dataclasses-gen-hash-action branch from 9c3bff4 to 4ce0344 Compare October 3, 2019 23:05
@ioistired ioistired marked this pull request as ready for review October 3, 2019 23:05
@ioistired ioistired requested a review from ericvsmith as a code owner October 3, 2019 23:05
@ericvsmith ericvsmith self-assigned this Oct 4, 2019
@ericvsmith
Copy link
Member

I'm going to reject this PR. See my rationale in the BPO issue.

@ericvsmith ericvsmith closed this Oct 4, 2019
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.

4 participants