-
Notifications
You must be signed in to change notification settings - Fork 428
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
fix: default font writer for character classification dataset #418
Conversation
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.
Thanks for the PR! Considering the user is supposed to be the only one to know which fonts are installed on her/his computer, I don't think it's a good idea to move forward with this PR (cf. my 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.
I added a suggestion to fix the CI and get a better user error/warning message
Codecov Report
@@ Coverage Diff @@
## main #418 +/- ##
==========================================
+ Coverage 95.80% 95.84% +0.03%
==========================================
Files 96 96
Lines 3937 3945 +8
==========================================
+ Hits 3772 3781 +9
+ Misses 165 164 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Thanks! I added a last improvement suggestion, but it's optional
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.
Thanks!
This PR replaces the default PIL font to FreeMono, to avoid the font size issue and UnicodeEncode issue.
Closes both #417 and #416
Any feedback is welcome!