Skip to content

Add ruff linter - #3417

Open
hunhoffe wants to merge 18 commits into
mainfrom
add-ruff-linter
Open

Add ruff linter#3417
hunhoffe wants to merge 18 commits into
mainfrom
add-ruff-linter

Conversation

@hunhoffe

@hunhoffe hunhoffe commented Jul 24, 2026

Copy link
Copy Markdown
Collaborator

Description

I don't enjoy enforcing Python style constraints in PRs, so I'd like the CI to do it for me, Part 1.

Checklist

  • Tests pass locally, or the change is covered by CI
  • Docs / docstrings updated if the change is user-facing
  • Code is formatted (clang-format for C++, black for Python — see CONTRIBUTING)

hunhoffe and others added 7 commits July 24, 2026 14:03
pylint was configured via .pylintrc but never actually invoked anywhere in
CI (the "clang-tidy-pylint" job only ran pyright + clang-tidy), so ruff
replaces it as the actual Python linter: wired into the lint CI job, the
pre-push pre-commit hook, and requirements_dev.

Co-Authored-By: Claude <noreply@anthropic.com>
Ruff's F401/isort passes kept breaking on real (if fragile) ordering
dependencies rather than false positives, which turned out to be actual
architectural problems worth fixing rather than working around:

- aie.utils.hostruntime.hostruntime and aie.helpers.taplib.{tensortiler2d,
  visualization2d} imported `tensor`/`ceildiv` back through
  aie.utils.__init__ instead of the module that actually defines them.
  Extracted tensor factories + NPU-backend selection into a new
  aie.utils.tensor_factory (and logging setup into aie.utils._log_setup,
  imported first for its side effect), so those modules import the real
  source directly. aie.utils.__init__ no longer needs a special import
  order.

- aie.iron.algorithms.{for_each,_transform,reduce} imported ObjectFifo/
  Program/Runtime/Worker/ExternalFunction back through aie.iron itself
  instead of from ..dataflow/..program/..runtime/..worker/..kernel. Fixed
  the same way; aie.iron.__init__ no longer needs special import order
  either.

- aie.iron.controlflow's only content (`_for as range_`) looked unused to
  ruff and would have been deleted by --fix; it's a re-export consumed by
  algorithms/for_each.py etc. Added `__all__` to mark it and every other
  similarly-shaped __init__.py re-export across the tree.

- compilabledesign.py's `_generate_mlir(self, ExternalFunction)` and
  compile/utils.py's `try: from aie.iron.kernel import ExternalFunction
  except ImportError` looked like circular-import workarounds but weren't
  (verified empirically: importing aie.iron.kernel at module level in
  both files works fine). Replaced with plain top-level imports.

- Two wildcard imports (txn2mlir/main.py, helpers/dialects/func.py) turned
  out to pull in only 2 symbols and 0 symbols respectively; converted to
  an explicit import and deleted the dead one.

Verified end-to-end against a real build (aie.iron, aie.utils, and the
algorithms submodules all import and run cleanly).

Co-Authored-By: Claude <noreply@anthropic.com>
Mechanical cleanup surfaced by the new ruff lint pass: unused imports,
unsorted imports, None/True/False comparison style, ambiguous single-letter
variable names, a bare except, an unused-variable, and E501 line wraps for
the handful of lines that were genuine code rather than the established
one-line-per-arg docstring convention (which the raised line-length limit
already covers). Also renames event_ir.py's internal camelCase locals to
snake_case to match the rest of the codebase.

Co-Authored-By: Claude <noreply@anthropic.com>
- python/CMakeLists.txt: register the new utils/_log_setup.py and
  utils/tensor_factory.py sources (split out of utils/__init__.py by
  the ruff pass) so the build actually copies them; their absence
  caused every build/test job to fail with a circular-import
  ImportError on aie.utils.
- compiler/txn2mlir/main.py: pyright ignore-comments must sit on the
  exact line pyright reports for a multi-line import.
- helpers/taplib/tas.py: the ruff rewrite of lambda-assigned
  offset/sizes/strides_fn into nested defs shadowed the parameter
  names, which pyright flags as a redeclaration; resolve into
  differently-named locals via a small constant-fn helper instead.
- iron/algorithms/{_transform,for_each,reduce}.py: switch to absolute
  aie.iron.* imports so pyright resolves Device/TensorAccessPattern
  through the same module identity used elsewhere in the codebase
  instead of a relative-import path that pyright treats as distinct.
- utils/jit.py: split_params lives in _introspect.py; the ruff pass
  removed compilabledesign.py's now-unused re-export of it, which
  broke this import.
# Conflicts:
#	python/iron/program.py
#	python/iron/runtime/runtime.py
#	python/utils/compile/jit/compilabledesign.py
ruff flagged the merge-conflict resolution's import order in
iron/program.py; sort get_compile_arg alongside the other aie.utils.* import.
Same root cause as the earlier jit.py fix: split_params lives in
_introspect.py, not compilabledesign.py. The ruff pass removed
compilabledesign.py's now-unused re-export of it, which broke this
test's import and failed every CI job that runs the lit test suite
(test/python/test_markers.py), even after the CMakeLists fix resolved
the aie.utils circular import.
@github-actions

Copy link
Copy Markdown
Contributor

Coverage Report

Created: 2026-07-25 00:37

Click here for information about interpreting this report.

FilenameFunction CoverageLine CoverageRegion CoverageBranch Coverage
Totals- - - -
Generated by llvm-cov -- llvm version 18.1.3

@hunhoffe hunhoffe left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems good. Most changes minor, except a few import changes which should make it harder to get circular import changes in the future.

@hunhoffe
hunhoffe marked this pull request as ready for review July 26, 2026 17:47

@andrej andrej left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we still need black with this?

@hunhoffe

hunhoffe commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator Author

@andrej Yep! Typical python open source stack is:

  • formatting (black)
  • linting (ruff, compatible with black)
  • static type checking (pyright or mypy)

@hunhoffe
hunhoffe added this pull request to the merge queue Jul 27, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 27, 2026
@hunhoffe
hunhoffe added this pull request to the merge queue Jul 27, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 27, 2026
@hunhoffe
hunhoffe added this pull request to the merge queue Jul 27, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 27, 2026
@hunhoffe
hunhoffe added this pull request to the merge queue Jul 28, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 28, 2026
@hunhoffe
hunhoffe enabled auto-merge July 28, 2026 16:09
hunhoffe added 6 commits July 28, 2026 14:44
Resolves conflicts from PR #3387 (Rework IRON Runtime as an eager
callback body), which replaced the old Runtime.sequence() context-
manager API with a callback-body design after this branch's ruff
cleanup commits landed:

- iron/runtime/runtime.py, iron/controlflow.py: take main's rewritten
  implementation wholesale (this branch's changes to the old API no
  longer apply); reorder imports to satisfy ruff/isort.
- iron/__init__.py, iron/runtime/__init__.py: merge the new Runtime
  exports (TaskGroup, RuntimeData, Task, sync_parameters) into the
  already-sorted import/__all__ lists from this branch.
- iron/algorithms/_transform.py, iron/worker.py,
  utils/compile/jit/compilabledesign.py: merge new imports
  (TaskGroup, flatten_fn_args, StringAttr) into this branch's
  absolute-import style, dropping duplicates.
- iron/runtime/task.py: drop an unused Buffer import that no longer
  applies now that DMATask-related subclasses moved out of this file.

Also ruff --fix + reorder imports in iron/dataflow/objectfifo.py and
iron/runtime/_context.py, which were added/touched by #3387 after this
branch's ruff pass and had never been linted.
Commit f59ce34 ("Fix circular imports and unnecessary import
indirection") replaced a try/except-guarded import of ExternalFunction
here with a plain top-level one, verified safe against main at the
time. #3387's Runtime rework since restructured aie/iron/__init__.py's
import order, and a top-level `from aie.iron.kernel import
ExternalFunction` now triggers aie.iron's __init__, which imports back
into aie.utils.compile.jit -- deadlocking on a cold aie.utils.compile
entry point (e.g. test_cache_file_lock.py importing
aie.utils.compile.cache.utils directly). Reproduced in the CI run for
8ed47ba: "ImportError: cannot import name 'NPU_CACHE_HOME' from
partially initialized module 'aie.utils.compile'".

Defer the import to its one call site, matching this file's existing
lazy-import pattern for the same class of cycle (see
resolve_target_arch's AIEArch import).
The previous fix (deferring ExternalFunction's import to its call site
in compile/utils.py) sidestepped the cycle instead of removing it, and
was itself order-dependent -- fragile the same way the bug it fixed
was. This does the structural fix instead.

BaseKernel/Kernel/ExternalFunction (iron/kernel.py) and Resolvable
(iron/resolvable.py) have no real iron-specific dependencies: they only
touch aie.ir/aie.dialects/aie.extras/aie.helpers, plus one isinstance()
check against iron.Buffer used purely to unwrap a resolved handle to
its underlying MLIR value. Move both to aie/utils/ (kernel.py,
resolvable.py) -- the same leaf layer aie.utils.callabledesign and
aie.utils.jit already occupy without cycling back into aie.iron -- and
replace the Buffer isinstance check with a duck-typed check for a
class-level `.op` attribute (checked on the class, not the instance,
so it never invokes an unresolved handle's raising property getter),
following the `.op`-unwrap convention already used by every other
resolvable handle in this codebase.

iron/kernel.py and iron/resolvable.py become thin backwards-compatible
re-exports (mirroring the existing iron/compile/__init__.py shim for
the same class of problem), so none of the ~30 existing call sites
across iron/, utils/compile/, tests, and programming_examples need to
change, and `aie.iron.ExternalFunction is aie.utils.kernel.ExternalFunction`
holds -- it's the same class object, not a copy.

compile/utils.py and compile/jit/compilabledesign.py now import
ExternalFunction/Kernel from aie.utils.kernel directly, so
aie.utils.compile.* no longer imports anything under aie.iron.* at
module scope in either file -- confirmed by an AST sweep of python/utils/**
for module-level (non-function, non-TYPE_CHECKING) `aie.iron` imports
(none found). The dependency graph is now a DAG: aie.iron -> aie.utils.compile.jit
-> aie.utils.kernel, never the reverse.

Verified against a real build (install/python, compiled bindings):
reproduced the original ImportError against the pre-fix utils.py,
confirmed it's gone after this change, and confirmed cold imports of
aie.utils.compile.cache.utils / aie.utils.compile / aie.utils.compile.jit
and aie.iron all succeed. Ran test/python (542 tests, excluding
npu-xrt/npu-hrx hardware suites) plus test_cache_file_lock.py (the
test that failed in CI) directly against the compiled bindings -- all
pass.
An earlier merge-conflict resolution combined this file's two aie.ir
imports (Module, StringAttr) into one statement with a single
# pyright: ignore[reportMissingImports]; ruff's isort pass then split
them back into two statements (it won't keep an aliased `X as Y` import
merged with a plain one) but only carried the ignore comment to the
first, silently leaving the second unresolvable-in-CI import
unguarded. CI's pyright job caught it: compilabledesign.py:44:6 - error:
Import "aie.ir" could not be resolved.
The aie.iron <-> aie.utils.compile cycle is resolved by the deferred
import in compile/utils.py (commit 3dbc5f3). The follow-on
restructure that moved BaseKernel/Kernel/ExternalFunction and Resolvable
into aie.utils (3eb8de1) was over-engineered and broke CI: the two
new aie/utils/*.py files were never registered in python/CMakeLists.txt,
so a clean build never staged them and every test failed with
"ModuleNotFoundError: No module named 'aie.utils.kernel'".

Revert those files to their pre-restructure state: the classes live in
aie.iron.kernel / aie.iron.resolvable again, and compile/utils.py keeps
the deferred `from aie.iron.kernel import ExternalFunction` at its call
site. Also retain the pyright ignore comment on the split aie.ir import
in compilabledesign.py (from 7a14e62), which is orthogonal to the
restructure and still needed.
@hunhoffe
hunhoffe added this pull request to the merge queue Jul 29, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants