Static code analysis

[rdesparbes]

Hello!

I find this project very interesting, and I am very impressed with the work that has been achieved here.

I think the project would greatly benefit from setting up some sort of static code analysis for the Python part:

• Type checking with mypy, pyright...
• Lint with ruff, pylint...

A CI that runs the tests would be pretty hard to set up because of the necessity to have a running CAD app in the environment, but static analysis would be easier to set up, and would provide some guarantees on the code and the .pyi files.

I already investigated mypy and ruff locally, and managed to find and fix some issues with the generation of the .pyi files, using these tools.

What do you think about it?

I am willing to work on this, but I want to be sure that we share the same views on this and that we choose the most adapted tools for that

[gswifort]

I think it's a good idea. It would be worth writing guidelines for contributors as well.

[CEXT-Dan]

It would be worth writing guidelines for contributors as well.

I thought about this. Started jotting down some ideas. I’ll need your guidance when it comes to Python

[CEXT-Dan]

It could be helpful, IMHO runtime analysis would be more useful, allow me to explain.

The main issue with .PYI files is that the output from boost::python is not PEP compliant
boostorg/python#277

The stubs are now generated by Grzegorz’s awesome generator. Here’s an example of the raw output from the C++ module

@command
def doit():
    print(Ax.AcadTable.setCellGridLineWeight.__doc__)


# setCellGridLineWeight( (AcadTable)arg1, (int)arg2, (int)arg3, (AcCellEdgeMask)arg4, (AcLineWeight)arg5) -> None :
#     ![(self, row:int, col:int, mask:PyAx.AcCellEdgeMask, lw:PyAx.AcLineWeight)]!<[{-1}]>
#     C++ signature :
#         void setCellGridLineWeight(class PyAcadTable {lvalue},int,int,enum PyAcCellEdgeMask,enum PyAcLineWeight)

output:

    def setCellGridLineWeight(self, row:int, col:int, mask:PyAx.AcCellEdgeMask, lw:PyAx.AcLineWeight, /) -> None:
        pass

The point of failure is that I hand enter the signatures inside C++ and inject them into the raw output ( inside the ![(… )]! markers). So linting might help shed light on some issues, they won’t tell us if it’s actually correct, unless it’s comparing with the C++ source, which is best done at runtime

IMHO, A useful tool might.
1, compare the number of arguments
2, look for duplicate argument names
3, check if the types are correct

I found I probably need to go back and fully qualify argument names, me using ‘val’ is not much better than ‘arg1’, ‘arg2’

[rdesparbes]

Ruff would be the easiest to set up:

1. ❌Ruff has no knowledge of the C++ implementation, so it cannot compare the number of arguments
2. ✅It detects invalid Python syntax, so duplicated arguments would be reported
3. ✅It can verify that the types are valid Python symbols that are defined in the file or imported

I am working an a PR to set it up with GitHub: #261

[rdesparbes]

@CEXT-Dan Hum, it looks like ruff has a tendency to remove imports in pyrx/pyrx_imp.py: a5db6dc#diff-15a60884d387b4c7cb7d2eb955b10b313dde92e9bc998010be87ce3a7602dba3

Is this file still needed? If I completely remove this file, I do not see any Pylance errors in VSCode, and PyCharm does not complain either.

[gswifort]

#262 (comment)

[gswifort]

@rdesparbes
The error in 5690aa8 is probably because mypy in CI works on linux and CREATE_NO_WINDOW is only on windows. How to deal with this? # type: ignore[attr-defined] seems unjustified.

gen_pyi.py:64: error: Module has no attribute "CREATE_NO_WINDOW"  [attr-defined]
gen_pyi.py:76: error: Module has no attribute "CREATE_NO_WINDOW"  [attr-defined]
gen_pyi.py:96: error: Module has no attribute "CREATE_NO_WINDOW"  [attr-defined]

[rdesparbes]

Maybe a constant could be defined?

CREATION_FLAGS = getattr(subprocess, "CREATE_NO_WINDOW", 0)

[gswifort]

Or probably mypy --platform win32

[gswifort]

Or probably mypy --platform win32

now it works: 08435c0