Deprecate int as interpolation argument type #5974
Conversation
|
@kylematoba Could you please provide sufficient information on the PR title/description etc? Thanks! |
kylematoba
changed the title
|
Yeah, sorry! It's done :). |
There was a problem hiding this comment.
@kylematoba There are some failing tests: https://app.circleci.com/pipelines/github/pytorch/vision/17435/workflows/8288ec5a-32c3-4d55-bb01-def2c5f2139c. From what I see, they all check the warning message emitted if the functions are called with an int argument. You probably only need to change the expected message there.
|
@kylematoba there seem to be a lot of unrelated formatting changes. These can distract the reviewer from what's relevant and they'll add some noise to git blame. Would you mind reverting these? If you'd like, you could also submit a PR that exclusively addresses these. |
test/test_functional_tensor.py
Outdated
| @@ -69,29 +69,12 @@ class TestRotate: | |||
| @pytest.mark.parametrize("device", cpu_and_gpu()) | |||
| @pytest.mark.parametrize("height, width", [(7, 33), (26, IMG_W), (32, IMG_W)]) | |||
| @pytest.mark.parametrize( | |||
| "center", | |||
There was a problem hiding this comment.
Although I appreciate this cleanup, it is unrelated to the PR. Could you please revert this? We will happily accept a follow-up PR with the changes.
|
Sorry about the mis-formatting. As far as I can see, it's undone and I think that the test failures are not related to my changes. |
|
cool, so can we merge this? |
| For backward compatibility integer values (e.g. ``PIL.Image[.Resampling].NEAREST``) are still acceptable. | ||
|
|
||
| For backward compatibility integer values (e.g. ``PIL.Image[.Resampling].NEAREST``) are still accepted, | ||
| but deprecated since 0.13 and will be removed in 0.15. Please use InterpolationMode enum. |
There was a problem hiding this comment.
I would be in favour of removing from the documentation the deprecation notes and leave them only as warnings. This will help us keep clean our docs and promote the best practices/options. I would leave the warnings as-is to inform those who already use it to change.
What I describe above is exactly what we currently do for Multi-weight API. The documentation no longer mentions anything about pretrained. Only about weights.
@NicolasHug thoughts?
There was a problem hiding this comment.
The pretrained case is a bit special because we actually removed it plainly, while keeping BC as much as possible. But in general I'm in favour of documenting deprecations in the docstrings as well
|
Hey @datumbox! You merged this PR, but no labels were added. The list of valid labels is available at https://github.com/pytorch/vision/blob/main/.github/process_commit.py |
Summary: * Requested here #5898 (comment). * Fix tests * ufmt, not black Reviewed By: NicolasHug Differential Revision: D36760927 fbshipit-source-id: 8ed8c6eaf32928f71ba6d4b3cf23a4b84f310d81 Co-authored-by: Philip Meier <[email protected]> Co-authored-by: Vasilis Vryniotis <[email protected]>
@pmeier asked me as part of #5898 to deprecate int as an accepted 'interpolation' argument type.