Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses Ruff linting rule RUF069 warnings by adding suppression comments and reorganizes imports in loopy/target/execution.py to improve type checking organization.
Changes:
- Adds
# noqa: RUF069suppressions to 7 test assertions to silence warnings about float literal clarity and division operations - Moves type-checking-only imports to the
TYPE_CHECKINGblock inloopy/target/execution.py - Adds explicit type annotations to
ExecutorBaseclass attributes
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
| test/test_target.py | Adds 4 RUF069 suppressions for float literals with trailing decimal points (2., 7., 5.) |
| test/test_statistics.py | Adds 2 RUF069 suppressions for integer expressions using true division operator |
| test/test_c_execution.py | Adds 1 RUF069 suppression for integer expression using true division operator |
| loopy/target/execution.py | Reorganizes imports: moves Expression, ArrayArg, _ArraySeparationInfo to TYPE_CHECKING block, corrects auto import source, adds Set import, and adds type annotations to ExecutorBase attributes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| assert a.get() == 1 | ||
| assert b.get() == 2. | ||
| assert b.get() == 2. # noqa: RUF069 |
There was a problem hiding this comment.
Instead of suppressing RUF069, consider fixing the float literal. Replace 2. with 2.0 for better readability and clarity. The RUF069 rule exists to ensure float literals are written in a clear, consistent style.
| _evt, out = knl(queue, y=y, x=x) | ||
| assert out[0] is x | ||
| assert x[0] == 7. | ||
| assert x[0] == 7. # noqa: RUF069 |
There was a problem hiding this comment.
Instead of suppressing RUF069, consider fixing the float literal. Replace 7. with 7.0 for better readability and clarity. The RUF069 rule exists to ensure float literals are written in a clear, consistent style.
| _evt, out = knl(queue, y=y, x=x) | ||
| assert out[0] is x | ||
| assert x[0] == 7. | ||
| assert x[0] == 7. # noqa: RUF069 |
There was a problem hiding this comment.
Instead of suppressing RUF069, consider fixing the float literal. Replace 7. with 7.0 for better readability and clarity. The RUF069 rule exists to ensure float literals are written in a clear, consistent style.
| _evt, out = knl(queue, y=y, x=x) | ||
| assert out[0] is x | ||
| assert x[0] == 5. | ||
| assert x[0] == 5. # noqa: RUF069 |
There was a problem hiding this comment.
Instead of suppressing RUF069, consider fixing the float literal. Replace 5. with 5.0 for better readability and clarity. The RUF069 rule exists to ensure float literals are written in a clear, consistent style.
| assert len(sync_map) == 2 | ||
| assert sync_map.filter_by(kind="kernel_launch").eval_and_sum(params) == 1 | ||
| assert sync_map.filter_by(kind="barrier_local").eval_and_sum(params) == 2*m/bsize | ||
| assert sync_map.filter_by(kind="barrier_local").eval_and_sum(params) == 2*m/bsize # noqa: RUF069 |
There was a problem hiding this comment.
Instead of suppressing RUF069, consider clarifying the division intent. If integer division is intended, use // operator (2*m//bsize). If float division is intended, consider using a float literal (2.0*m/bsize) to make the intent explicit.
| assert sync_map.filter_by(kind="barrier_local").eval_and_sum(params) == 2*m/bsize # noqa: RUF069 | |
| assert sync_map.filter_by(kind="barrier_local").eval_and_sum(params) == 2*m//bsize |
|
|
||
| # (count-per-sub-group)*n_subgroups | ||
| assert local_mem_s == m*2/bsize*n_subgroups | ||
| assert local_mem_s == m*2/bsize*n_subgroups # noqa: RUF069 |
There was a problem hiding this comment.
Instead of suppressing RUF069, consider clarifying the division intent. If integer division is intended, use // operator (m*2//bsize*n_subgroups). If float division is intended, consider using a float literal (m*2.0/bsize*n_subgroups) to make the intent explicit.
| assert local_mem_s == m*2/bsize*n_subgroups # noqa: RUF069 | |
| assert local_mem_s == m*2//bsize*n_subgroups |
| target=lp.ExecutableCTarget())(n=n) | ||
|
|
||
| assert out == (n*(n-1)/2) | ||
| assert out == (n*(n-1)/2) # noqa: RUF069 |
There was a problem hiding this comment.
Instead of suppressing RUF069, consider clarifying the division intent. If integer division is intended, use // operator (n*(n-1)//2). If float division is intended, consider using a float literal (n*(n-1)/2.0) to make the intent explicit.
| assert out == (n*(n-1)/2) # noqa: RUF069 | |
| assert out == (n*(n-1)//2) |
1 participant
No description provided.