search

claird / PyPDF4

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

PyPDF4/filters.py unit tests #9

Closed acsor closed 5 years ago

acsor commented 5 years ago

This is an ongoing PR - do not close it right after the first merge, I will continue pushing more until I say I am done with it. In the meantime, review and eventually merge the existng code.

I have continued adding unit tests for filters in PyPDF4/filters.py, and admittedly they turned out quite useful in estirpating a few bugs or refactoring some algorithms. As of now, I'm not fully sure that LZWDecode.Decoder is bug-free as I found it in the codebase prior to working on it; indeed, one of its test cases keeps failing.

There were also a few other commits not fully related to unit testing, but they were good enough to be added.

acsor commented 5 years ago

Commit 169dce1 should end issue #8. There's only one staticmethod() direct call but it is not proper to modify.

acsor commented 5 years ago

Hey @claird, isn't it possible to set Pylint flags such as these in the source code:

class CCITTFaxDecode(object):    # pylint: disable=too-few-public-methods
    @staticmethod

in the configuration of the Pylint tool? I will check myself, and if you agree will weed out those invasive comment declarations.

claird commented 5 years ago

Pylint indeed respects a .pylintrc.

As I understand .pylintrc, if I mention too-few-public-methods there, it will turn off that check for all methods. I want to leave the checks on for at least a while longer. I understand your point that static methods don't deserve the stigma of a source #pylint declaration. While I suspect there's a good solution for that issue, I haven't researched it yet.

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

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

Hey @claird https://github.com/claird, isn't it possible to set Pylint flags such as these in the source code:

class CCITTFaxDecode(object): # pylint: disable=too-few-public-methods @staticmethod

in the configuration of the Pylint tool? I will check myself, and if you agree will weed out those invasive comment declarations.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/claird/PyPDF4/pull/9#issuecomment-415747994, or mute the thread https://github.com/notifications/unsubscribe-auth/AAbN9KZCQV8zzCeiqe6SHUUPADh3FXNrks5uT_WAgaJpZM4WISzQ .

acsor commented 5 years ago

I had been prolonging Add fixes to Sample_Code/*.py. for too long, and I decided to include it in this PR even if not intimately related. (It is a very small modification to an unessential part of the project.)

acsor commented 5 years ago

This is an ongoing PR - do not close it right after the first merge

I am noting that GitHub doesn't allow reopening PRs that were just merged. Will need to find a workaround or just resort to opening a new one each time.

acsor commented 5 years ago

Hey @claird, since PRs cannot be reused on GitHub you can merge the changes in #9 and close it.

claird commented 5 years ago

Thank you, newnone, for your contributions and patience.