Questions related to Ruff

[j-t-1]

Feel free to discuss any of the three (without answering them all), the numbering is just for reference.

1. Complexity of the code base is a broad subject and has different measures. Restricting this to the below measure of complexity, do we know the functions which are greater than 10, and especially the ~54?

[tool.ruff.lint.mccabe]
max-complexity = 54  # Recommended: 10

2. Shall we add this (convention to be decided) to pyproject.toml:

[tool.ruff.lint.pydocstyle]
convention = "google"

3. Shall we change setup.cfg so it uses ruff:

[darglint]
enable=DAR201
ignore=DAR002

[stefan6419846]

1. There is lots of legacy code and/or complex cases which need to be considered. I have already proposed to use code in such a modular fashion in the past that we can run unit tests on artificial data in a more suitable way instead of having to rely on real PDF files which are sometimes hard to generate. (This does not mean that we should support issues about such bugs without a reproducing example.)

   To determine the "most complex" code, just reduce the value in the configuration and run ruff on your local checkout of the code base.

2. Does enabling this setting have any effect within enabling pydocstyle checks? If you enable this, how many valid and invalid violations do you see?

3. The setup.cfg should be dropped altogether and migrated to pyproject.toml where necessary.

   • mutmut seems to be unused and at the moment, nobody has executed on the whole code base due to taking too long (see Review mutmut configuration #2525).
   • check-wheel-contents is never being installed or executed by us, thus it seems to be obsolete?
   • darglint is unmaintained and not used by our CI. If this currently is not covered by ruff, we should check and enable the corresponding rules.

[j-t-1]

Nice information @stefan6419846.

[tool.ruff.lint.mccabe]
max-complexity = 10

Gives 48 errors, and the top two are _extract_text (52) and test_extract_text_visitor_callbacks (37).

I have already proposed to use code in such a modular fashion in the past that we can run unit tests on artificial data in a more suitable way instead of having to rely on real PDF files which are sometimes hard to generate.

This would have benefits of being able to either copy or point to when there are issues or questions.

2. Only one error was reporterd, have implemented this as DEV: Add tool.ruff.lint.pydocstyle to pyproject.toml #3059.

3. Will be good migrate setup.cfg to pyproject.toml. I think tool.ruff.lint.pydocstyle can replace darglint, although am not an expert on these tools.

[stefan6419846]

Judging from https://github.com/terrencepreilly/darglint, we had two checks enabled:

• DAR002: An argument/exception lacks a description
• DAR201: The docstring is missing a return from definition.

Skimming through the ruff docs, DAR201 should be covered by https://docs.astral.sh/ruff/rules/undocumented-param/ and DAR002 by https://docs.astral.sh/ruff/rules/undocumented-public-class/ I recommend you to verify this with some invalid cases locally before cleaning this up.

Please note that I am not completely sure about this as I did not want to read/analyze the full darglint code.

[j-t-1]

Because of ignore=DAR002, we only have to check:

DAR201: The docstring is missing a return from definition.

A test for a function without a return type triggered the Ruff rule ANN202: Missing return type annotation for private function.

[stefan6419846]

ANN202 is not sufficient for DAR201 - we probably need the whole ANN2* set.

[j-t-1]

We have two ANN2* excluded Ruff rules.

ANN201: Missing return type annotation for public function. 608 errors, all in the tests folder.

ANN204: Missing return type annotation for special method. 18 errors.

I will try and fix the ANN204 rules.

[stefan6419846]

ANN201 is safe to ignore in the tests. ANN204 sounds like something we could ignore as well as all special methods have a known return value per definition.

[j-t-1]

Now have understood _extract_text better it does have some nice features. Using a dataclass to replace the global variables could be the way to go and then maybe move the nested process_operation function outside of the function.

[stefan6419846]

Do we really need this complexity in an intermediate step? What I would have proposed as the easiest approach:

• Convert _extract_text into a class with the main logic inside a method in there.
• Move the global state management into class attributes.
• Move the nested functions into dedicated methods inside the class.

[j-t-1]

Better! The main logic is already present in the function.