-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
STY: Refactor b_ #2772
STY: Refactor b_ #2772
Conversation
Comment courtesy of @shartzog, issue py-pdf#2726.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2772 +/- ##
==========================================
- Coverage 95.16% 95.16% -0.01%
==========================================
Files 51 51
Lines 8548 8545 -3
Branches 1704 1703 -1
==========================================
- Hits 8135 8132 -3
Misses 261 261
Partials 152 152 ☔ View full report in Codecov by Sentry. |
I am not sure if this comment really is required here instead of just linking to the corresponding thread for example. |
I wanted it self-contained, and not needing to refer externally. The comment contains all the information needed to understand why we are caching characters. |
I'm not sure about the comment : When you "print" character per character they are all within the same content stream. still not sure the caching is done the good way |
@pubpub-zz I have removed the comment in b_ for now. One idea is we could pre-compute the dictionary B_CACHE for the 256 values, using utf-8 as the encoding. So just have a lookup table and one if statement testing if the length of the string is one. Also, how about a rename of b_ to bytes_ as this is more self-describing and would match the converse str_? |
Comment courtesy of @shartzog, issue #2726.