Skip to content

Add _FilterArgs NamedTuple to ImageFilter - #9968

Open
radarhere wants to merge 2 commits into
python-pillow:mainfrom
radarhere:filter
Open

Add _FilterArgs NamedTuple to ImageFilter#9968
radarhere wants to merge 2 commits into
python-pillow:mainfrom
radarhere:filter

Conversation

@radarhere

Copy link
Copy Markdown
Member

See if you think this makes the code clearer.

@akx akx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I like the idea. Minor comments - things I had to look up to understand the flow, so maybe someone else would have to, too :)

Comment thread src/PIL/ImageFilter.py

class _FilterArgs(NamedTuple):
size: tuple[int, int]
scale: float

@akx akx Sep 7, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Kernel calls it scale.

def __init__(
self,
size: tuple[int, int],
kernel: Sequence[float],
scale: float | None = None,
offset: float = 0,
) -> None:

I would rather be consistent with the existing Python API.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Sure thing.

Comment thread src/PIL/ImageFilter.py
pass


class _FilterArgs(NamedTuple):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd kind of want to note that these are unpacked directly to ImagingCore.filter().

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ok, I've added a comment.

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