Intersection of PdfReader and PdfWriter

[j-t-1]

It may be good to decrease the symmetric difference of PdfReader and PdfWriter functionality.

Two examples:

1. PdfWriter has generate_file_identifiers, but PdfReader does not have file_identifiers?
2. PdfWriter has open_destination, but PdfReader does not?

For 1. what is the current way of reading the file identifiers?
For 2. is there a reason for this?

[stefan6419846]

In #2497, we started working on unifying the code between both classes, thus some oddities might be resolved in the future.

There probably is not much to tell about the reasoning for why one class does it like this and the other class another way. Primarily write-specific functionality might be available inside the writer only, which makes sense.

With #2495, the reader got the _ID property you were asking for - although this might break without warning due to not being part of the public API.

[j-t-1]

Kudos for already having done most of this. If there is a list of potential things still to unify these two could be appended.

[j-t-1]

One thing that may help us is reordering the methods to be in similar.

PdfWriter has is_encrypted as its first member, and this would be slightly better further down (like PdfReader), since then __init__ would be top, which some prefer.

[pubpub-zz]

Do not hesitate to propose a PR

[j-t-1]

Have started this, with #2654; likely will do more reordering to increase consistency further.

Method repr_mimebundle is now last as it goes less with the others. Could it be renamed to make it more self-documenting?

[stefan6419846]

I recommend you to have a look at the external docs in case of _repr_mimebundle_ in this case. This should indicate that the name is part of an external protocol and thus cannot be changed.

[j-t-1]

Thanks, did not know this.

[j-t-1]

The __init__ of PdfWriter has the line:
self._objects: List[PdfObject] = []

What is the PdfReader equivalent of _objects?

The __init__ of PdfReader has the line:
self.resolved_objects: Dict[Tuple[Any, Any], Optional[PdfObject]] = {}

But this does not start with an underscore, starts with "resolved_" and has different types.

[pubpub-zz]

_objects and resolved_objects are completely different : _objects stores the object to be written whereas resolved_objects (so far I rememer) stores the objects already extracted.
In order to ease traceability vs time I've tried as much as possible too keep the existing (before the PR) names

[j-t-1]

In PdfWriter we have:

def close(self) -> None:
    """To match the functions from Merger."""
    return

1. Shall I change the docstring to (what I think is intended) """Close file descriptor"""?
2. Is it correct, at the moment it does not do anything?

[stefan6419846]

Both the old and your proposed docstring are wrong in my opinion. As long as we do not close any file descriptors there, this is just a dummy implementation for interface harmonization and should be documented as such.

[pubpub-zz]

• Shall I change the docstring to (what I think is intended) """Close file descriptor"""?

the file descriptor is open/closed in .write()

• Is it correct, at the moment it does not do anything?

the function was create to have the same API as PdfMerger The text is correct.

[j-t-1]

Because PdfMerger will be removed, we could make the message more generic, like " Implemented for interface harmonization" or "Implemented for API harmonization"?

[pubpub-zz]

sounds good to me

[j-t-1]

_rolling_checksum is used by PdfWriter._compute_document_identifier.

Is it worth changing it to use the current time, a string representation of the PDF file’s location, and the size of the PDF file in bytes (§14.4 File identifiers).

Also, as it is not used elsewhere, we could make it a method and move it to the bottom of PdfWriter?

[stefan6419846]

There already has been #2003 which proposed an alternative way to generate the file identifiers, but not yet merged, thus I am not sure whether it makes sense to propose another way here.

[j-t-1]

Thanks. The PDF specification way is also non-deterministic until time travel becomes feasible #2003 (comment). It is easy to implement (let me know if you want a PR) and we could have an argument for an alternative deterministic option. The current way is not so good for big files #2003 (comment).

[stefan6419846]

If ever, this should be discussed/evaluated separately, not inside this general discussion.