-
-
Notifications
You must be signed in to change notification settings - Fork 887
Do not use static options for default quantizer constructors. #1721
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
Codecov Report
@@ Coverage Diff @@
## master #1721 +/- ##
=======================================
Coverage 84.40% 84.41%
=======================================
Files 822 822
Lines 36056 36063 +7
Branches 4203 4207 +4
=======================================
+ Hits 30434 30443 +9
Misses 4802 4802
+ Partials 820 818 -2
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
antonfirsov
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, thanks!
|
Wait ... should we do the same checks in |
Yuuuuuuuup. Missed that, good catch! Done. |
Prerequisites
Description
While investigating #1583 I realized that the static default options in all the quantizer instances could be overwritten which we should not allow.
I've also ensured we throw a better exception should someone use the default current struct instances to dither.