Skip to content

DOC: Use AnnotationFlag enum instead of plain integers - #3997

Merged
stefan6419846 merged 2 commits into
py-pdf:mainfrom
RavSinghChandan:fix-annotation-flags-setter
Aug 24, 2026
Merged

stefan6419846 merged 2 commits into
py-pdf:mainfrom
RavSinghChandan:fix-annotation-flags-setter

Conversation

@RavSinghChandan

Copy link
Copy Markdown
Contributor

AnnotationFlag is an IntFlag, so naming both it and int 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 and asserts it reads back.

Only the setter widens; the getter still returns AnnotationFlag.

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

codecov Bot commented Aug 21, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.98%. Comparing base (4c8f707) to head (57e6959).
⚠️ Report is 3 commits behind head on main.

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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@stefan6419846 stefan6419846 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.
@RavSinghChandan

Copy link
Copy Markdown
Contributor Author

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.

@stefan6419846 stefan6419846 changed the title STY: Accept a plain int when setting annotation flags DOC: Use AnnotationFlag enum instead of plain integers Aug 24, 2026
@stefan6419846
stefan6419846 merged commit 08e6f6b into py-pdf:main Aug 24, 2026
52 of 63 checks passed
stefan6419846 pushed a commit that referenced this pull request Aug 25, 2026
…ags (#4003)

update_page_form_field_values takes its flags as FieldDictionaryAttributes
FfBits, and the docstring points readers at that enum, but the tests passed a
bare 1 in five places. FfBits(1) is ReadOnly, so spelling it out says what the
call is asking for.

Same shape as #3997.
stefan6419846 added a commit that referenced this pull request Sep 4, 2026
## 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)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants