xla icon indicating copy to clipboard operation
xla copied to clipboard

Add .pylintrc Configuration File

Open apivovarov opened this issue 1 year ago • 5 comments

This PR introduces a .pylintrc file to the XLA project.

XLA, which is used in TensorFlow, relies on TensorFlow's CI builds to automatically test its code, including Python components like xla_client. Recently, issues were encountered with TensorFlow's CI reporting errors such as pylint C0301: Line too long in xla_client_test.py.

TF CI links:

  • https://github.com/tensorflow/tensorflow/commit/dd2c1e9d94cfe5c471b5788917a2f74140d9b144
  • https://github.com/tensorflow/tensorflow/actions/runs/11130353052/job/30929504884

I found that XLA lacked a dedicated Pylint configuration (e.g., .pylintrc or pyproject.toml), which made it harder to automatically receive proper warnings and errors, particularly when working in editors like VSCode.

To address this, I have added a .pylintrc file to the XLA project. This file is based on TensorFlow's configuration, with modifications made to remove or fix unsupported configuration lines.

apivovarov avatar Oct 01 '24 21:10 apivovarov

I don't know much about pylint. @ddunl can you review this?

reedwm avatar Oct 01 '24 22:10 reedwm

I'll have to investigate what we do internally, an explicit goal I have for XLA is that the CI maps directly to what we do internally as often as possible (this is a work in progress, and sometimes is not possible). I'll have to see what we do internally. It's not totally clear to me that what TF does here is correct, because I don't think that on e.g. a PR to TF, that this pylint config is enforced in any meaningful way. I'll look into this and get back to you

ddunl avatar Oct 01 '24 22:10 ddunl

JAX utilizes the pyproject.toml file, which includes the [tool.ruff.lint] section and other Ruff configurations. It no longer uses Pylint.

However, since the XLA development workflow is closely integrated with TensorFlow CI, we need to adhere to TensorFlow's Pylint rules. This approach helps us avoid spending valuable time resolving XLA PR merge issues reported by TensorFlow CI.

apivovarov avatar Oct 01 '24 22:10 apivovarov

TF and JAX behave differently in this regard, TF may have that pylint job on PRs, but internally the status of the pylint on the PR is irrelevant - only the internal linter checks are respected. JAX does what I would prefer XLA do which is check both. I'm not exactly sure the mechanism by which JAX keeps the internal config in sync with the external one. One starting point I'd be interested in is to use https://google.github.io/styleguide/pylintrc and see if that currently agrees with all code we have checked in

ddunl avatar Oct 01 '24 23:10 ddunl

TF pylintrc reports about 80 issues in xla/python

Styleguide pylintrc reports about 1000 issues

apivovarov avatar Oct 01 '24 23:10 apivovarov