Skip to content

Fix deprecated affine parameters#299

Merged
cregouby merged 13 commits intomlverse:mainfrom
Chandraveersingh1717:fix-deprecated-affine-parameters
Apr 9, 2026
Merged

Fix deprecated affine parameters#299
cregouby merged 13 commits intomlverse:mainfrom
Chandraveersingh1717:fix-deprecated-affine-parameters

Conversation

@Chandraveersingh1717
Copy link
Copy Markdown
Contributor

Replace deprecated parameter names in transform functions

The transform functions were using resample and fillcolor parameters, which are deprecated in PyTorch's API. This updates them to use interpolation and fill instead to match the current PyTorch conventions.

What changed

  • Renamed resample to interpolation in rotate and affine transforms
  • Renamed fillcolor to fill in affine transforms
  • Added backward compatibility so old code still works

Functions updated

  • transform_rotate
  • transform_affine
  • transform_random_rotation
  • transform_random_affine

Backward compatibility

Old parameter names still work but show a warning message:

transform_rotate(img, angle = 45, resample = 0) # works, shows warning
transform_rotate(img, angle = 45, interpolation = 0) # recommended

@Chandraveersingh1717
Copy link
Copy Markdown
Contributor Author

Please review this

@cregouby
Copy link
Copy Markdown
Collaborator

Hello @Chandraveersingh1717,

Thanks for this proposal.
todo Could you please remove from this proposal all file changes unrelated to Fix deprecated affine parameters ?
(i.e. do not stack changes on top of each other, but create new branch from a clean main each time).
suggestion : To check your P.R., please have a loook at Files Changes tab at the top of your P.R.

I'll be happy to review it then.

@Chandraveersingh1717 Chandraveersingh1717 force-pushed the fix-deprecated-affine-parameters branch from 5da4c8b to 1a2d3c6 Compare March 23, 2026 07:02
@Chandraveersingh1717 Chandraveersingh1717 force-pushed the fix-deprecated-affine-parameters branch from b793f64 to 286ee7d Compare March 24, 2026 19:19
Copy link
Copy Markdown
Collaborator

@cregouby cregouby left a comment

Choose a reason for hiding this comment

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

praise thanks a lot for this effort, alignement with pytorch/vision API is a driver for this project.
todo see inline comments
todo Please run document(), and check() from devtools before submitting a P.R.

Comment thread NAMESPACE
Comment thread R/transforms-generics.R Outdated
transform_random_rotation <- function(img, degrees, interpolation = 0, expand=FALSE,
center=NULL, fill=NULL, resample) {
if (!missing(resample)) {
warning("'resample' is deprecated, use 'interpolation' instead")
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.

todo please do not use warning() but rather one of the functions in conditions.R such as deprecated() in order for package translation to work.

Comment thread R/transforms-defaults.R
Comment thread R/transforms-defaults.R
Comment thread R/transforms-generics.R
Comment thread R/vision_utils.R
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.

todo generated artifact Please remove commented line of equal sign
question why did you reduce the function documentation down to something limited to informed users ?
suggestion blocking please revert the function parameter documentation
todo You cannot remove exemples in function documentation. Please revert

Comment thread tests/testthat/test-transforms.R Outdated
Comment on lines +201 to +214
warn_msgs <- character(0)
old <- withCallingHandlers(
transform_affine(x, 0, c(0, 1), 1, 0, resample = 0, fillcolor = 0),
warning = function(w) {
warn_msgs <<- c(warn_msgs, conditionMessage(w))
invokeRestart("muffleWarning")
}
)

new <- transform_affine(x, 0, c(0, 1), 1, 0, interpolation = 0, fill = 0)

expect_equal(sum(grepl("resample", warn_msgs)), 1)
expect_equal(sum(grepl("fillcolor", warn_msgs)), 1)
expect_equal_to_r(old, as_array(new))
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.

todo readability & consistency please rely on the much simpler expect_warning() is you expect a warning from a function call.

@cregouby cregouby merged commit 442326c into mlverse:main Apr 9, 2026
@Chandraveersingh1717
Copy link
Copy Markdown
Contributor Author

Thanks for the updates! I’ve checked the changes and have a much clearer idea of the expected direction now. I’ll make sure to align my future changes with this.

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.

3 participants