Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pep8 Conformance #12

Open
stonier opened this issue Mar 18, 2014 · 1 comment
Open

Pep8 Conformance #12

stonier opened this issue Mar 18, 2014 · 1 comment

Comments

@stonier
Copy link
Member

stonier commented Mar 18, 2014

@jihoonl, @bit-pirate, @dwlee, @hughie, @piyushk

This is carried over from Piyush's work initiated in the multimaster repo (robotics-in-concert/rocon_multimaster#259). It's useful here as a reminder when creating new or updating old rocon python packages.

Roslint

All python packages in the rocon repositories should conform to pep8. To enable automated testing:

  • Add a build dependency on roslint:
<build_depend>roslint</build_depend>
  • Enable roslint macros and call them:
find_package(catkin REQUIRED COMPONENTS roslint)
...
# Lint Python modules for PEP8 compatibility
file(GLOB_RECURSE ${PROJECT_NAME}_PY_SCRIPTS
     RELATIVE ${PROJECT_SOURCE_DIR} scripts/*)
file(GLOB_RECURSE ${PROJECT_NAME}_PY_SRC
RELATIVE ${PROJECT_SOURCE_DIR} src/${PROJECT_NAME}/*.py)
roslint_python(${${PROJECT_NAME}_PY_SCRIPTS})
roslint_python(${${PROJECT_NAME}_PY_SRC})
  • Test pep8 conformance using:
catkin_make roslint_<package_name>

To automatically cut down a lot of errors:

sudo pip install autopep8
autopep8 --max-line-length 120 --in-place scripts/*
autopep8 --max-line-length 120 --in-place src/<package-name>/*.py

You can also try the --aggresive for more aggresive correction. See autopep8 --help.

Roslint Tips

  • Ignoring a line of code for analysis - add a # noqa comment at the end of the line.

This should in general be avoided and appears to be a contentious issue in the python world. The main argument against it is that it creates noise in the source file. I'm agnostic about it and care more just to get on with our work so do as you wish. One use case for it is in formatting a set of lines with extra whitespace for ease of reading (and hence arguably creating less noise in the source file despite the comment).

@stonier stonier added this to the ToDo milestone Mar 18, 2014
@stonier
Copy link
Member Author

stonier commented Mar 18, 2014

Eclipse - PyDev

  • pep8 settings are in Preferences->Pydev->Editors->Code Analysis.
  • It uses its own internal pep8 script
  • This is a bit older than the roslint pep8 script and not so configurable (e.g. no --max-line-length)
  • You can't redirect it to the new pep8 scripts (e.g. roslint's) either (jpython incompatibilities)

So we can't get pydev running the same pep8 as roslint. However I haven't noticed any big difference except the default line length (pydev: 80, roslint: 120). I generally try and conform to that too these days, but you can always set --ignore=E501 in the eclipse settings and let roslint handle it on the command line.

@stonier stonier modified the milestones: Discussion, ToDo Mar 18, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant