claird / PyPDF4

A utility to read and write PDFs with Python
obsolete-https://pythonhosted.org/PyPDF2/
Other
328 stars 61 forks source link

Rename encode/decode classes in filters.py #10

Closed acsor closed 5 years ago

acsor commented 5 years ago

Many, if not all, of the classes in PyPDF4/filters.py have an uncorresponding nomenclature (and a very ugly one indeed), for the operations they perform. Those classes I refer to are ones as FlateDecode, ASCIIHexDecode, LZWDecode etc., but other than simply decoding data, they also encode it in the format they represent. (Some of them have just started doing it in PR #9, where I have included encode() methods for the easing out of unit testing.)

I propose to refactor these classes as *Codec; for example FlateCodec, ASCIIHexCodec and so on, for the above-mentioned reason that they cover both operations, encoding and decoding, most of the time.

I have done a preliminary usage test and they are referenced in only a limited fraction of the codebase (most of the time one or two references, possibly in the same source file). Furthermore, the official PyPDF2 documentation has never documented them to the public and I'm left thinking that the external code that depends on them is very limited.

This is a straightforward modification but requires thinking a bit about it in group. But as of now, the names of these classes are almost totally unrepresentative of what they do.

claird commented 5 years ago

Oscar, I support the renaming you propose: to the best of my knowledge, it will help us.

Cameron Laird, vice president We make computers work for people.

On Fri, Aug 24, 2018 at 8:15 AM, Oscar notifications@github.com wrote:

Many, if not all, of the classes in PyPDF4/filters.py have an uncorresponding nomenclature (and a very ugly one indeed), for the operations they perform. Those classes I refer to are ones as FlateDecode, ASCIIHexDecode, LZWDecode etc., but other than simply decoding data, they also encode it in the format they represent. (Some of them have just started doing it in PR #9 https://github.com/claird/PyPDF4/pull/9, where I have included encode() methods for the easing out of unit testing.)

I propose to refactor these classes as *Codec; for example FlateCodec, ASCIIHexCodec and so on, for the above-mentioned reason that they cover both operations, encoding and decoding, most of the time.

I have done a preliminary usage test and they are referenced in only a limited fraction of the codebase (most of the time one or two references, possibly in the same source file). Furthermore, the official PyPDF2 documentation https://pythonhosted.org/PyPDF2/index.html has never documented them to the public and I'm left thinking that the external code that depends on them is very limited.

This is a straightforward modification but requires thinking a bit about it in group. As of now, the names of these classes are almost totally unrepresentative.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/claird/PyPDF4/issues/10, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbN9CyzbCZzElP6vZmmBzoM1NT9HUg-ks5uT_x5gaJpZM4WLVXY .

acsor commented 5 years ago

I have already started the work, but I'm setting up a test case to ensure that the transition is smooth and error-free.

claird commented 5 years ago

Super! Thank you, Oscar.

Cameron Laird, vice president We make computers work for people.

On Sun, Sep 9, 2018 at 4:07 PM, Oscar notifications@github.com wrote:

I have already started the work, but I'm setting up a test case to ensure that the transition is smooth and error-free.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/claird/PyPDF4/issues/10#issuecomment-419745103, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbN9Pxs-phE7PIjlAFi8rwHzNBbn7Ghks5uZYMTgaJpZM4WLVXY .

acsor commented 5 years ago

Closed with #14.