Add ResizingMode.Fill to ImageResizerTransformer #2362
Conversation
|
@mareklinka Can you please unskip locally RegenerateEntryPointCatalog test, run it, and push changes in core_ep-list.tsv? |
| @@ -710,5 +711,59 @@ public void TestBackAndForthConversionWithoutAlphaNoInterleaveNoOffset() | |||
| Done(); | |||
| } | |||
| } | |||
|
|
|||
| [Fact] | |||
| public void ImageResizerTransform_ResizingModeFill() | |||
There was a problem hiding this comment.
ImageResizerTransform_ResizingModeFill [](start = 20, length = 38)
Some of us have strong feelings regarding Python_style_of_defining_names, can you forgive us, and change it to C# style of ImageResizerTransformResizingModeFill ? #Resolved
There was a problem hiding this comment.
No problem 😄 #Resolved
Is it possible the test only changed |
Rename test
| var leftMid = bitmap.GetPixel(bitmap.Width / 3, bitmap.Height / 2); | ||
| var rightMid = bitmap.GetPixel(bitmap.Width / 3 * 2, bitmap.Height / 2); | ||
|
|
||
| Assert.All(new[] { topLeft, topRight, bottomLeft, bottomRight, middle }, c => |
There was a problem hiding this comment.
Assert.All(new[] { topLeft, topRight, bottomLeft, bottomRight, middle }, c => [](start = 14, length = 79)
The joy of multi environment support. Mac OS and Linux fails because some of values become 254 instead of 255.
There was a problem hiding this comment.
Hm, weird. Some kind of rounding error, I guess. I'll rewrite the test to be a little more permissive.
|
Sorry, sometimes I forget which one is which. If you have changes in arguments it's manifest if you have changes in class names it's list. In reply to: 459814339 [](ancestors = 459814339) |
Codecov Report
@@ Coverage Diff @@
## master #2362 +/- ##
==========================================
+ Coverage 71.17% 71.25% +0.07%
==========================================
Files 780 783 +3
Lines 140404 140781 +377
Branches 16053 16088 +35
==========================================
+ Hits 99936 100309 +373
+ Misses 36018 36017 -1
- Partials 4450 4455 +5
|
| @@ -44,7 +44,10 @@ public enum ResizingKind : byte | |||
| IsoPad = 0, | |||
|
|
|||
| [TGUI(Label = "Isotropic with Cropping")] | |||
| IsoCrop = 1 | |||
| IsoCrop = 1, | |||
|
|
|||
There was a problem hiding this comment.
Would be great if you could add a summary tag above these options, this will make it show in the documentation online and in the intellisense for future users.
| /// <summary> | |
| /// Ignore aspect ratio and squeeze/stretch into target dimensions. | |
| /// </summary> |
There was a problem hiding this comment.
Looks great! Thank you for doing this @mareklinka
Would be great if you could one last thing so that this option appears both in the online API documentation and in intellisense.
| @@ -44,7 +44,10 @@ public enum ResizingKind : byte | |||
| IsoPad = 0, | |||
|
|
|||
There was a problem hiding this comment.
| /// <summary> | |
| /// Isotropic with cropping. | |
| /// </summary> |
| @@ -44,7 +44,10 @@ public enum ResizingKind : byte | |||
| IsoPad = 0, | |||
There was a problem hiding this comment.
| IsoPad = 0, | |
| /// <summary> | |
| /// Isotropic with padding. | |
| /// </summary> | |
| IsoPad = 0, |
|
Thanks again! |
ghost
locked as resolved and limited conversation to collaborators
This PR adds a new resizing mode to the
ImageResizerTransformer. This mode is calledFilland when used, the transformer ignores aspect ratio and squeezes/stretches the source image into the target dimensions.The idea for this mode came up in #2022 - when attempting to score a Keras-trained model it turned out that Keras resizes images in a way that ML.NET cannot do on its own. To increase interoperability, the Fill mode was proposed.
I also included a test for the new mode. This tests uses a specially prepared image, runs the resizing transformation, and then checks whether the result fulfills the specification for the resizing mode.
Fixes #2022