ENV : clarification about building

[pubpub-zz]

@MartinThoma,
To prevent too many cosmetic commits can you please re-clarify what are the command line parameters and config file used during generation.
Also, do you have some reference/recommendation about codevod to propose the best coverage : I was expecting more pdf input files to cover coverage, but not sure it's working

[pubpub-zz]

converted to discussion

[MartinThoma]

Sure!

The simplest stylistic changes I make are done be pre-commit. You can do those automatically with every commit by running once

pip install pre-commit

# Within the PyPDF2 git root folder:
pre-commit install 

# it should show something like 'pre-commit installed at .git/hooks/pre-commit'

For existing PRs, you can run pre-commit run --all. Common changes are:

• Apply the black formatter
• Apply isort for sorting imports
• Ensure files end with a newline

[MartinThoma]

Simple, but not easily automatable:

• PEP8 naming: The flake8-plugin pep8-naming points out simple PEP8 issues, but we still have the functions/methods/parameters that got deprecated. So there are quite a lot of messages
• Private/public API: Keeping the public interface (anything that does not start with an underscore) small is desirable as we can easily change things. If we modify the behavior / name of anything of the public interface, we have to go through the deprecation process to ensure we're not unexpectedly breaking other peoples code.

Complex:

• Code structure: I don't want functions / classes / modules to become too big. On the one hand, it makes them hard for me to understand. On the other hand, there might be more merge conflicts and the merge conflicts become more complicated.

[MartinThoma]

do you have some reference/recommendation about codevod to propose the best coverage

On a more general note about testing: I'm currently downloading the Tika test dataset of 800,000 PDF files. Well, a part of it as my laptop doesn't have enough storage 😅 I plan to run simple operations over all of those files to ensure that we don't crash.

The other path I'm trying to go is to increase test coverage to 100% and kill all mutants found by the mutmut mutation tester (intro to mutation testing if you're curious).

Increasing test coverage is typically also easier if you have smaller functions. Then you just test the function - you might not even need a PDF for that. Those test typically also run faster, but there is a higher risk that you're testing something that cannot happen / that the test itself is not so good. Mutation testing improves the quality of the tests.

[pubpub-zz]

@MartinThoma , @MasterOdin
I don't know if I'm right, but black seems to be configured to work with 80 char-long lines. I've always considered that this recommandation is obsolete with your high resolution/wide screens. Shouldn't we consider to reconfigure black to 120 ?

[MasterOdin]

black is configured to use 88 char long lines, with flake8 configured to ignore line length errors (E501).

I personally like 88 (with trying to be around 80) as it allows me to open two files next to each other and see the entire line on my 16" display.

[MartinThoma]

I work on a 14-inch laptop (with super high resolution, but still) and I'm also reading code on my phone. I also like to view diff on Github.

The 88 char limit of black seems good to me; in some of my private projects I set the limit to 79 characters. It works mostly fine when you don't use Java-esque variable names and avoid too many nested layers (which you should due to mental complexity)

[xilopaint]

I would replace flake8 with Pylint. It takes some time to get rid of all the messages but it worth it in my opinion.

[MartinThoma]

There is no reason we have to decide between Flake8 or pylint. We can use both.

I haven't put pylint in the CI because it gives way too many messages at the moment.

[pubpub-zz]

May I suggest to restore label "Help Wanted" in github in order to try to get attention of other developpers on some issues?

[pubpub-zz]

#1091
your idea about difficulty sound good however do you think that the label will be reported at higher level like for "want help" ?

[MartinThoma]

The labels will probably not shown like this.

Are you suggesting to put the "help wanted" label on easy issues to encourage people to start contributing?

[pubpub-zz]

yes

[MartinThoma]

Makes sense to me - I'll activate that when I'm back home.

Thank you for the suggestion 😊

[MartinThoma]

I've added the "help wanted" back + added the topic "help-wanted" to the project: https://github.com/topics/help-wanted?l=python&o=desc&s=stars