ROB : update_page_form_field_values not failing if no fields #1346
Conversation
Codecov ReportBase: 94.62% // Head: 94.71% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1346 +/- ##
==========================================
+ Coverage 94.62% 94.71% +0.08%
==========================================
Files 30 30
Lines 5118 5185 +67
Branches 1052 1061 +9
==========================================
+ Hits 4843 4911 +68
Misses 164 164
+ Partials 111 110 -1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
PyPDF2/_writer.py
Outdated
| if PG.ANNOTS not in page: | ||
| return |
There was a problem hiding this comment.
Shouldn't we add the attribute if it doesn't exist, e.g.:
| if PG.ANNOTS not in page: | |
| return | |
| if PG.ANNOTS not in page: | |
| page[PG.ANNOTS] = DictionaryObject() |
There was a problem hiding this comment.
if the form has no fields to field, adding the PG.ANNOTS field may be of no use and quite confusing after (looking at the page structure you may think after there is so fields to fill up whereas there isn't)
There was a problem hiding this comment.
if the form has no fields to field
I guess there was a typo? I don't understand this.
At the very least we should emit a warning ... if we are in strict mode maybe even an exception?
There was a problem hiding this comment.
if the form has no fields to be filled, adding the PG.ANNOTS field may be of no use and quite confusing after (looking at the page structure you may think after there is so fields to fill up whereas there isn't) ; more consistent with PDF philosophy : if you don't need it, it is not in the dictionary (no fields means logically no PG.ANNOTS entry)
There was a problem hiding this comment.
I understand why you don't want to add it. It makes sense to me now.
However, looking at it from the users perspective: The user wants to fill a form field that doesn't exist. PyPDF2 executes sucessfully without an exception, without a warning, without a log message. No field is filled. That feels wrong to me.
A warning log message - something like "Filling fields was skipped as the page does not have any annotations" would give the user a chance to fix the code.
|
wrongly deleted |
|
@MartinThoma |
|
Thank you :-) I've just released, but it will be in the next release :-) |
New Features (ENH): - Addition of optional visitor-functions in extract_text() (#1252) - Add metadata.creation_date and modification_date (#1364) - Add PageObject.images attribute (#1330) Bug Fixes (BUG): - Lookup index in _xobj_to_image can be ByteStringObject (#1366) - \'IndexError: index out of range\' when using extract_text (#1361) - Errors in transfer_rotation_to_content() (#1356) Robustness (ROB): - Ensure update_page_form_field_values does not fail if no fields (#1346) Testing (TST): - read_string_from_stream performance (#1355) Full Changelog: 2.10.9...2.11.0
fixes #1343