Add _FilterArgs NamedTuple to ImageFilter - #9968
Conversation
akx
left a comment
There was a problem hiding this comment.
I like the idea. Minor comments - things I had to look up to understand the flow, so maybe someone else would have to, too :)
|
|
||
| class _FilterArgs(NamedTuple): | ||
| size: tuple[int, int] | ||
| scale: float |
There was a problem hiding this comment.
I'd personally like to call this divisor, because that's what it practically is (and in fact, that's the "argument name" in _filter() in _imaging.c).
There was a problem hiding this comment.
Kernel calls it scale.
Lines 72 to 78 in 5eee920
I would rather be consistent with the existing Python API.
| pass | ||
|
|
||
|
|
||
| class _FilterArgs(NamedTuple): |
There was a problem hiding this comment.
I'd kind of want to note that these are unpacked directly to ImagingCore.filter().
There was a problem hiding this comment.
Ok, I've added a comment.
Merging this PR will improve performance by 89.2%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| ⚡ | test_filter[1237x811-L-Smooth] |
53.1 ms | 16.5 ms | ×3.2 |
| ⚡ | test_filter[1237x811-L-Sharpen] |
52.9 ms | 16.5 ms | ×3.2 |
| ⚡ | test_filter[1237x811-I;16-Sharpen] |
153.3 ms | 137.3 ms | +11.64% |
| ⚡ | test_filter[1237x811-I;16-Smooth] |
153.4 ms | 137.5 ms | +11.63% |
Tip
Curious why performance improved? Comment @codspeedbot explain why performance improved on this PR, or directly use the CodSpeed MCP with your agent.
Comparing radarhere:filter (749c66e) with main (e7f9779)2
Footnotes
-
335 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
-
No successful run was found on
main(3c7080c) during the generation of this report, so e7f9779 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩
See if you think this makes the code clearer.