Python Linting

[stonier]

We've got some custom python linting infrastructure here and over in maliput_py. Ostensibly we should have some in delphyne_demos as well.

Custom Machinery Hotfix

$ colcon test --packages-select delphyne --abort-on-error --event-handlers console_direct+
...
test 1
      Start  1: PYLINT

1: Test command: /home/snorri/workspace/src/delphyne/tools/run_python_checks.sh
1: Test timeout computed to be: 9.99988e+06
1: Error: pycodestyle was not found in your system.
1: Please install it by running:
1: 
1: pip install --user pycodestyle

The run_python_checks.sh tool checks for the program pycodestyle which is installed via the pycodestyle deb (not the python-/python3- prefixed libraries. It's the correct one we need as it depends on the python3 version. To fix:

• Add pycodestyle to https://github.com/ros/rosdistro/blob/master/rosdep/python.yaml
• Add pycodestyle to package.xml
• Drop the instructions for pip install, should be relying on debs if there isn't a good reason
• Repeat and rinse for maliput_py

Eliminating the Custom Machinery

Not sure what the reasons for the custom machinery were (maybe the ament provided machinery was not yet ready), but there are 3 packages we could use in ament_lint - ament_pep257, ament_pyflake, ament_pycodestyle. We could/should investigate switching over to these.

[stonier]

Oh IIRC, at one point (a couple of ros2 versions ago) the OSRC build farm was using custom versions of this tooling for their CI. I did submit a complaint at the time - this makes it hard for users to make PR's against their core repos if the system debians for build tooling signalled by rosdep just happen to not be satisfactory - you can't run the tests, nor do you know what custom version is required so you can get on with it.

That might or might not still be true with dashing.

[francocipollone]

Eliminating the Custom Machinery Not sure what the reasons for the custom machinery were (maybe the ament provided machinery was not yet ready), but there are 3 packages we could use in ament_lint - ament_pep257, ament_pyflake, ament_pycodestyle. We could/should investigate switching over to these.

Indeed, integrating this is quite straight forward. See doc: ament_cmake_pycodestyle/doc/

[agalbachicar]

Per chat and research with @francocipollone , ament_pycodestyle is not released for dashing. We will include it into the workspace but it won't be maintained by us).

+1 to use the new available machinery. It was not there at the moment.

[francocipollone]

Next steps:

• ~Add ament_cmake_pycodestyle to dsim.repos~
• Use ~ament_pycodestyle~ ament_cmake_flake8 and remove old python check machinery in:
  • maliput_py
  • maliput_integration
  • delphyne
  • drake_vendor

[francocipollone]

We can't just add ament_cmake_pycodestyle to dsim.repos file, we should bring the entire ament_lint package which brings many not-used-by-us packages.

There is one simpler and cleaner option. (thanks @agalbachicar). ament_cmake_flake8 is available in dashing. We can deprecate the pycodestyle script check and replace it with ament flake8 machinery. The outcome will be in essence the same.

I will proceed that way.

[agalbachicar]

Closing this ticket as everything was already addressed.