-
Notifications
You must be signed in to change notification settings - Fork 51
refactor!: Improve performance across the project #128
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
Conversation
|
Thanks for the great performance improvements. One note: switching several constants from list to frozenset and renaming them introduces a breaking change for users who import these constants directly. Could you add a short note here describing the intention behind the constant renaming and the expected impact on users? |
It is a good point, I completely forgot about compatibility when was working on it. I have reverted changes in constant names, but keep them as frozensets and add a note about breaking changes in PR description. |
|
Also updated hand calculation logic a bit to calculate yaku only when it is necessary, it added +2% performance gain on default hand config (and didn't introduce breaking changes) |
# Conflicts: # tests/tests_utils.py
|
@Apricot-S thanks for thoughtful review. I have addressed your comments and also added breaking changes notes to changelog.md. Once this branch is merged, please add notes to the same file about breaking changes that were introduced in your other PRs |
Apricot-S
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I started to work on #125 and resurrected the code for hands validation.
So far I checked on 2kk hands and no regressions were found. Before the release 2.0 I will check on more hands and will update readme with this info.
While I was here I did some profiling and few optimizations there and there to speed up lib a bit. They were low hanging fruits that can be done without sacrificing readability much and without big refactorings.