Make invalid k-means options an %Image.Error{} + return {:ok, model} from Image.Scholar.k_means/2

CHANGELOG.md

@@ -30,6 +30,10 @@
 
 * **Breaking:** `Image.chroma_mask/2`, `Image.chroma_key/2`, `Image.replace_color/2` and their `!` variants now return `{:error, %Image.Error{reason: :invalid_option}}` when options from both masking strategies are supplied, or when only one of `:greater_than` and `:less_than` is supplied. The two strategies were always documented as mutually exclusive, but nothing enforced it: the color range strategy won and `:color` and `:threshold` were silently discarded, and a lone bound was discarded and replaced by auto chroma detection with the default threshold. ([#224](https://github.com/elixir-image/image/pull/224))
 
+* **Breaking:** `Image.Scholar.k_means/2` returns `{:ok, model}` instead of a bare fitted `Scholar.Cluster.KMeans` on success, matching `Image.Scholar.unique_colors/1` and the rest of the library. ([#227](https://github.com/elixir-image/image/pull/227))
+
+* **Breaking:** `Image.k_means/2` and `Image.reduce_colors/2` return `{:error, %Image.Error{reason: :invalid_option}}` for an invalid or unknown option instead of raising `NimbleOptions.ValidationError`. `Image.k_means!/2` and `Image.reduce_colors!/2` raise `Image.Error` rather than the NimbleOptions exception. `operation` is set to `k_means` or `reduce_colors`, and `value` is `{key, value}` for an invalid value or the list of keys for unknown options. ([#227](https://github.com/elixir-image/image/pull/227))
+
 * `Image.affine/3` and `Image.rotate/3` now premultiply alpha explicitly only when the background is non-opaque, since libvips handles the other cases itself. `Image.shear/4` and `Image.translate/4` inherit this. ([#217](https://github.com/elixir-image/image/pull/217))
 
 ### Fixed

lib/image.ex

@@ -8959,17 +8959,18 @@ defmodule Image do
 
       with {:ok, srgb_image} <- Image.to_colorspace(image, :srgb),
            {:ok, target_image} <-
-             Image.new(1, 1, color: :black, interpretation: original_colorspace) do
+             Image.new(1, 1, color: :black, interpretation: original_colorspace),
+           {:ok, model} <- Image.Scholar.k_means(srgb_image, options) do
         k_means =
-          srgb_image
-          |> Image.Scholar.k_means(options)
-          |> Map.fetch!(:clusters)
+          model.clusters
           |> Nx.to_list()
           |> Enum.map(fn rgb -> Enum.map(rgb, &round/1) end)
           |> Enum.sort()
           |> Enum.map(fn rgb -> Pixel.to_pixel!(target_image, rgb) end)
 
         {:ok, k_means}
+      else
+        {:error, reason} -> {:error, Image.Error.wrap(reason, operation: :k_means)}
       end
     end
 
@@ -9119,6 +9120,13 @@ defmodule Image do
     @doc subject: "Clusters", since: "0.50.0"
 
     def reduce_colors(%Vimage{} = image, options \\ []) do
+      case do_reduce_colors(image, options) do
+        {:error, reason} -> {:error, Image.Error.wrap(reason, operation: :reduce_colors)}
+        other -> other
+      end
+    end
+
+    defp do_reduce_colors(image, options) do
       with {:ok, image} <- to_colorspace(image, :srgb) do
         kmeans_num_clusters =
           Keyword.get(options, :colors, @default_clusters)
@@ -9136,16 +9144,15 @@ defmodule Image do
           |> to_nx!()
           |> Nx.reshape({height * width, bands})
 
-        model =
-          Scholar.Cluster.KMeans.fit(nx_reshaped, options)
-
-        indicies =
-          Nx.as_type(model.labels, :u8)
+        with {:ok, model} <- Image.Scholar.fit(nx_reshaped, options) do
+          indicies =
+            Nx.as_type(model.labels, :u8)
 
-        model.clusters
-        |> Nx.take(indicies)
-        |> Nx.reshape({height, width, bands})
-        |> Image.from_nx()
+          model.clusters
+          |> Nx.take(indicies)
+          |> Nx.reshape({height, width, bands})
+          |> Image.from_nx()
+        end
       end
     end
 

lib/image/scholar.ex

@@ -106,6 +106,37 @@ if match?({:module, _module}, Code.ensure_compiled(Scholar.Cluster.KMeans)) and
       %Image.Error{message: message, reason: message}
     end
 
+    # Scholar.Cluster.KMeans.fit/2 validates its options with
+    # NimbleOptions.validate!/2 which raises on an invalid or unknown
+    # option, so translate the exception to {:error, %Image.Error{}} here
+    # at the boundary
+    @doc false
+    def fit(samples, options) do
+      {:ok, Scholar.Cluster.KMeans.fit(samples, options)}
+    rescue
+      exception in NimbleOptions.ValidationError ->
+        {:error, invalid_option(exception)}
+    end
+
+    # An unknown option sets :key to the list of unknown keys and leaves
+    # :value nil. An invalid value sets both.
+    defp invalid_option(%NimbleOptions.ValidationError{key: keys} = exception)
+         when is_list(keys) do
+      %Image.Error{
+        reason: :invalid_option,
+        value: keys,
+        message: Exception.message(exception)
+      }
+    end
+
+    defp invalid_option(exception) do
+      %Image.Error{
+        reason: :invalid_option,
+        value: {exception.key, exception.value},
+        message: Exception.message(exception)
+      }
+    end
+
     @doc """
     Clusters the unique colors of an image using the K-means
     algorithm.
@@ -120,7 +151,8 @@ if match?({:module, _module}, Code.ensure_compiled(Scholar.Cluster.KMeans)) and
 
     ### Returns
 
-    * A fitted `Scholar.Cluster.KMeans` model or
+    * `{:ok, model}` where `model` is a fitted
+      `Scholar.Cluster.KMeans` or
 
     * `{:error, reason}`.
 
@@ -145,7 +177,7 @@ if match?({:module, _module}, Code.ensure_compiled(Scholar.Cluster.KMeans)) and
               options
           end
 
-        Scholar.Cluster.KMeans.fit(colors, options)
+        fit(colors, options)
       end
     end
 

test/image_analysis_coverage_test.exs

@@ -152,6 +152,24 @@ defmodule Image.AnalysisCoverageTest do
     test "reduce_colors!/2 returns an image", %{image: image} do
       assert %Vimage{} = Image.reduce_colors!(image, colors: 2, key: Nx.Random.key(1))
     end
+
+    test "k_means/2 returns an error for an invalid option value", %{image: image} do
+      assert {:error,
+              %Image.Error{
+                reason: :invalid_option,
+                operation: :k_means,
+                value: {:num_clusters, 0}
+              }} = Image.k_means(image, num_clusters: 0)
+    end
+
+    test "reduce_colors/2 returns an error for an unknown option", %{image: image} do
+      assert {:error,
+              %Image.Error{
+                reason: :invalid_option,
+                operation: :reduce_colors,
+                value: [:unknown_option]
+              }} = Image.reduce_colors(image, unknown_option: true)
+    end
   end
 
   describe "Image.preview/1 and Image.p/1" do