Codecov Report

Base: 92.28% // Head: 92.30% // Increases project coverage by +0.02% 🎉

Coverage data is based on head (8688ba3) compared to base (cc32b59).
Patch coverage: 97.50% of modified lines in pull request are covered.

@@            Coverage Diff             @@
##             main    #1611      +/-   ##
==========================================
+ Coverage   92.28%   92.30%   +0.02%     
==========================================
  Files          33       33              
  Lines        6376     6410      +34     
  Branches     1271     1280       +9     
==========================================
+ Hits         5884     5917      +33     
  Misses        312      312              
- Partials      180      181       +1     

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

ready for 3.5.0 😊

That's a pretty awesome new feature! I'm curious to try this out from a users perspective tomorrow!

Regarding the interface, I want to try a couple of things:

1. Can we reasonably make use of _utils.File?
2. Can we make this a property (e.g. simply attachments)?
3. Instead of (1), maybe something that stores the pointer / metadata, but does not actually read the part into memory (not sure if that would be much more complicated / if there would be value in it)
4. The name "attachment": How does EmbeddedFiles relate to file annotations? (I want to avoid running into a bookmark/outline-renaming kind of situation)

That's a pretty awesome new feature! I'm curious to try this out from a users perspective tomorrow!

☺️ this was expected a long time ago based on the issues...

Regarding the interface, I want to try a couple of things:

1. Can we reasonably make use of `_utils.File`?

That's feasable however not sure it will be very useful...

2. Can we make this a property (e.g. simply `attachments`)?

PdfReader will have only getter, and for memory usage, the list function will have to be kept.
for PdfWriter, there was no request was raised about a getter.

3. Instead of (1), maybe something that stores the pointer / metadata, but does not actually read the part into memory (not sure if that would be much more complicated / if there would be value in it).

The PDF will have been loaded in memory any way. but it will still be "compressed". I added an optional parameter to get only the file data you are looking for.

4. The name "attachment": How does `EmbeddedFiles` relate to file annotations? (I want to avoid running into a bookmark/outline-renaming kind of situation)

From my test, they are independant (although Acrobat Reader shows them in the attachment list).I agree embedded_files may be clearer
on the other hand there was add_attachment() so don't know what to think...

This is the file generated with two attachments:

out-attachment.pdf

The following readers seem not to be able to display this attachment / attachments at all:

• Google Chrome Version 109.0.5414.119 (Official Build) (64-bit)
• Okular Version 1.9.3

Atril Document Viewer 1.24.0

Document Viewer 3.36.10

Firefox

I'm happy with the implementation, but I'm uncertain about the public interface in the PdfReader.

For the reader, could it be an attribute attachments which maps to something like this:

class _DelayedDict:
    def __init__(self, list_attachments: Callable[[], List[str]], get_attachment: Callable[[str], bytes]]):
        self.list_attachments = list_attachments
        self.get_attachment = get_attachment

    def keys(self) -> List[str]:
        return self.list_attachments()
    
    def __len__(self) -> int:
        return len(self.keys())

    def __getitem__(self, key: str) -> bytes:
        return self.get_attachment(key)

The idea is similar to the _VirtualList we use for the pages attribute. Just giving users something that feels / behaves in a familiar way.

One way to get this merged fast would be to postpone the decision on the public interface by re-naming:

• get_attachments -> _get_attachments
• list_attachments -> _list_attachments
  (I'm not saying you should do that ... I'm just uncertain how eager you are to get this into the release this weekend 😄 )

I've just completed with a commit for a test case I missed: multiple files with the same name (this case is acceptable for Acrobat Reader) : if multiple inputs have the same name, first they will be present many times in the list_attachments. In the dict returned by get_attachements they will be present in an array (in the same order as in the pdf attachment array

@MartinThoma
I've tried to refactor with your proposal, however, I've got a problem: get_attachments() return the full list but I can not get the same with attachments : attachments[] is incorrect; if i'm using get(), getitem() will be not be accessible. can we consider attachments[None] as acceptable ?