Repository navigation
DOC: Use AnnotationFlag enum instead of plain integers - #3997
Conversation
AnnotationFlag is an IntFlag, so naming both it and int in the annotation would be redundant. The setter wraps the value in a NumberObject and the getter already normalises through AnnotationFlag, so an int round-trips: test_annotation_dictionary sets 123 directly.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3997 +/- ##
=======================================
Coverage 97.98% 97.98%
=======================================
Files 57 57
Lines 11098 11098
Branches 2078 2078
=======================================
Hits 10874 10874
Misses 125 125
Partials 99 99 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
stefan6419846
left a comment
There was a problem hiding this comment.
Doesn't this cause confusion? I mean, we have this enumeration, thus I would prefer if users would pass proper enum values for better readability.
Reverted the annotation change. The documentation and the test both passed a bare int while the docs comment pointed readers at AnnotationFlag, so the readability gap was there rather than in the type. The example now sets AnnotationFlag.PRINT, which is the 4 it spelled out before, and the test combines three flags.
|
Agreed, and looking again the confusion was already in the documentation rather than the type. The example set annotation.flags = 4 directly under a comment telling the reader to see AnnotationFlag for the other options, so it pointed at the enum while demonstrating a magic number. I've reverted the annotation change and fixed that instead: the example now uses AnnotationFlag.PRINT, which is the 4 it spelled out before, and test_annotation_dictionary combines three flags rather than asserting on 123. pypdf/annotations/_base.py is untouched now. |
## What's new ### Security (SEC) - Limit value for Roman numerals (#4047) by @stefan6419846 ### New Features (ENH) - _cmap.py: Also parse encoding for embedded CFF Type1 fonts (#4032) by @PJBrs ### Performance Improvements (PI) - Cache repeated text extraction character lookups (#4036) by @petermik68-sudo ### Bug Fixes (BUG) - Treat an empty /Filter array as no filter when extracting images (#4026) by @Anai-Guo - Detect a duplicate dictionary key whose first value is falsy (#4024) by @devYRPauli - Make is_open=False collapse outline items (#3998) by @SomSamantray ### Robustness (ROB) - Multiple changes for wrong inputs by @RavSinghChandan - Skip trailing duplicate %%EOF markers when locating startxref (#4015) by @Anai-Guo - Do not crash on a non-array destination (#3976) by @eeshsaxena - Handle annotations without subtype during merge (#3999) by @Nexlu1 ### Documentation (DOC) - Use AnnotationFlag enum instead of plain integers (#3997) by @RavSinghChandan ### Code Style (STY) - Multiple small changes detected from test runs by @RavSinghChandan [Full Changelog](6.16.2...6.17.0)
AnnotationFlagis anIntFlag, so naming both it andintwould be redundant. The setter wraps the value in aNumberObjectand the getter already normalises throughAnnotationFlag, so an int round-trips —test_annotation_dictionarysets123directly and asserts it reads back.Only the setter widens; the getter still returns
AnnotationFlag.