Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.43%. Comparing base (bd95bd8) to head (86b9c82).
⚠️ Report is 2 commits behind head on main.

@@           Coverage Diff           @@
##             main    #3660   +/-   ##
=======================================
  Coverage   97.43%   97.43%           
=======================================
  Files          55       55           
  Lines       10022    10022           
  Branches     1842     1842           
=======================================
  Hits         9765     9765           
  Misses        149      149           
  Partials      108      108           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Sorry for coming back late. You said: "Please update the configuration to run proper linting in the CI for these files." I used ruff from the configuration files in the command line, but I fear that's not what you wanted exactly. Therefore can you please elaborate on what you mean by updating the configuration to run proper linting in the ci for these files.

Also you said: Additionally, running the type checker for files which are not guaranteed to have type hints at all does not sound correct as well. These should ideally be included into the type checks before getting started with the testing PR.

Can you also explain this a little bit more or give some examples or hints on which files you are talking about. That would be very much appreciated.

@stefan6419846

I used ruff from the configuration files in the command line, but I fear that's not what you wanted exactly.

Yes, because we want to automatically run this in CI, not only locally. Additionally, I guess you meant mypy as in the original comment, as ruff is the style checker, not the type checker.

Therefore can you please elaborate on what you mean by updating the configuration to run proper linting in the ci for these files.

The mypy configuration needs to be updated to reflect these changes, id est not exclude the migrated files any more. This most likely means updating the corresponding pyproject.toml sections.

Additionally, in the workflow file for the typing tests, we currently run mypy pypdf only. This needs to be updated to cover all correct files which are not explicitly excluded by the corresponding configuration section.

Also you said: Additionally, running the type checker for files which are not guaranteed to have type hints at all does not sound correct as well. These should ideally be included into the type checks before getting started with the testing PR.
Can you also explain this a little bit more or give some examples or hints on which files you are talking about. That would be very much appreciated.

In the pyproject.toml file, we explicitly exclude make_release.py from the usual type checks. Enabling type checks for the tests should enable them for the script itself as well to make sure that we do not miss anything.

I understand, however shouldn't this be at the last pr. This pr is only covering a small portion of the tests, in the last pr when i finish all the tests then we can include the tests folder in the configuration. Do you want me to include only the folders that i finished (for example tests/generic tests/scripts).

No, because I do not want to have to remember which parts have been migrated manually and ideally want to be able to check in CI that everything is correct.

Solutions:

1- I finish all the tests and add the tests folder into the configuration and ci in one pr which is this one.

or

2- If we have to split all the tests into several prs then we can't add the whole tests folder into the ci from the first pr since it will fail. Then we either can add the parts that are finished or ignore it till the last pr.

Your suggestions are much appreciated.

We should add files to the configuration as soon as we attempted the migration/fix.

Fixed the inline comments, I am not sure if i missed any of the inline comments you added. I also fixed the failing ci.
I added the missing yaml stub in the requirements file and regenerated them using the pip-compile as commented.
Now mypy also runs in the ci checking the test folders i finished.

I reverted the changes done by the black formatter manually hopefully didn't miss any.
Thank you for all the feedback.

@stefan6419846 Hope you are doing well, thank you for the help in this pr.
Is there anything i need to fix?

I am currently on vacation and thus reviews are mostly delayed, hoping that I find enough time next week.

Thank you for the guidance and the patience.