vision icon indicating copy to clipboard operation
vision copied to clipboard

Fix docstring for 'fill' in transforms without 'padding_mode'

Open abhitorch81 opened this issue 4 months ago • 8 comments

Summary

This PR applies the minimum docstring fix for fill, as discussed in #9149.

Replaced:

Pixel fill value used when the padding_mode is constant.

With:

Pixel fill value used for pixels outside the image boundary.

This corrects the docstring in transforms like ElasticTransform, RandomAffine, etc., which do not have a padding_mode parameter.

No functional changes made.

Fixes: #9149

Hi @diaz-esparza — no worries at all, and thank you for your thoughtful message!

I’ve gone ahead and applied the minimum docstring fix as discussed in the issue, specifically removing the misleading reference to padding_mode and updating the fill parameter description for clarity.

You’re of course very welcome to iterate or build on top of it — especially if you’re still considering the more advanced version as a feature enhancement.

Appreciate your openness to feedback — and honestly, your explanation and tone were already clear and friendly! Looking forward to your thoughts on the PR

abhitorch81 avatar Aug 07 '25 09:08 abhitorch81

:link: Helpful Links

:test_tube: See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/9171

Note: Links to docs will display an error until the docs builds have been completed.

This comment was automatically generated by Dr. CI and updates every 15 minutes.

pytorch-bot[bot] avatar Aug 07 '25 09:08 pytorch-bot[bot]

pixels outside the image boundary is a really nice way of putting it, props on that!

On that note though, please be careful with a few things about the change. Mainly, you've also altered the Pad and RandomCrop docstrings, and I think it's important on those two to keep the padding_mode reference given that the fill parameter is only useful when the other one is set to "constant". Both reverting the change or putting [...] for pixels outside the image boundary when the ``padding_mode`` is constant should be fine!

Also I think there's a misindentation on the RandomAffine docstring and an extra line in RandomCrop, please be on the lookout for that kind of stuff :)

diaz-esparza avatar Aug 07 '25 13:08 diaz-esparza

pixels outside the image boundary is a really nice way of putting it, props on that!

On that note though, please be careful with a few things about the change. Mainly, you've also altered the Pad and RandomCrop docstrings, and I think it's important on those two to keep the padding_mode reference given that the fill parameter is only useful when the other one is set to "constant". Both reverting the change or putting [...] for pixels outside the image boundary when thepadding_modeis constant should be fine!

Also I think there's a misindentation on the RandomAffine docstring and an extra line in RandomCrop, please be on the lookout for that kind of stuff :)

I will keep that in mind and update the PR.

abhitorch81 avatar Aug 08 '25 08:08 abhitorch81

Agreed with @diaz-esparza that we need to make sure that when there is a padding_mode parameter, we explicitly mention it in the docstring.

I will work on it.

abhitorch81 avatar Aug 29 '25 17:08 abhitorch81

I will do that.

abhitorch81 avatar Sep 04 '25 16:09 abhitorch81

Fixed the fill param docstring indentation

abhitorch81 avatar Sep 16 '25 07:09 abhitorch81

@pytorchbot rebase

abhitorch81 avatar Sep 20 '25 18:09 abhitorch81

You don't have permissions to rebase this PR since you are a first time contributor. If you think this is a mistake, please contact PyTorch Dev Infra.

pytorch-bot[bot] avatar Sep 20 '25 18:09 pytorch-bot[bot]