-
-
Notifications
You must be signed in to change notification settings - Fork 2k
Fixed Style hash #2346
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
Fixed Style hash #2346
Conversation
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #2346 +/- ##
==========================================
- Coverage 98.88% 98.71% -0.18%
==========================================
Files 73 73
Lines 7629 7761 +132
==========================================
+ Hits 7544 7661 +117
- Misses 85 100 +15
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
olivierphi
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 🙂
Fixes a performance bug in the Style class.
Essentially the hash calculation was inconsistent. Things that were equal would sometimes have different hashes. I noticed this when adding
lru_cachetoStyle.__add__. What should have been an easy win caused dramatically reduced performance, because it broken the mechanics of storing Styles in a dict.The hashing is now in once place. It is also lazy, so the hash is stored the first time it is used.