Fix #96: find functions fail with Not_found as documented

This commit ensures that `find` functions fail with `Not_found`
consistently with their documentation.

The inconsistency was introduced for `Py.Object.find`,
`Py.Object.find_string`, `Py.Dict.find` and `Py.Dict.find_string` by
commit af7e79a which replaced the dedicated function
`Pyutils.option_unwrap` by `Option.get`, which raises
`Invalid_argument _` instead of `Not_found`.

The inconsistency was present since the beginning for
`Py.Object.find_attr_string` and `Py.Object.find_attr`.

Functions `Py.Object.find_err`, `Py.Object.find_string_err`,
`Py.Object.find_attr_err`, `Py.Object.find_attr_string_err` have been
introduced for cases where keeping the underlying Python exception is
preferable.  For instance, `Py.Module.get` is now an alias for
`Py.Object.find_attr_string_err` to keep the current behavior of
failing with a Python exception.

Suggested by jonathan-laurent:
#96

CHANGES.md

@@ -4,23 +4,37 @@
 
 - Compatibility with Python 3.13
 
-- Fix segmentation fault by forgetting objects on library unloading
-  (observed on Fedora Rawhide with address randomization)
-  (reported by Jerry James,
-  https://github.com/thierry-martinez/pyml/issues/85)
+- Fix segmentation fault by forgetting objects on library unloading.
+  Observed on Fedora Rawhide with address randomization,
+  reported by Jerry James,
+  https://github.com/thierry-martinez/pyml/issues/85
 
 - #93, #94: Fix `Py.Object.get_attr_string`: this function now returns
   `None` when attribute is missing (the former version raised an
   exception, despite the `option` return type and contrary to what was
-  documented) (reported by Lindsay Errington, @dlindsaye,
-  https://github.com/thierry-martinez/pyml/issues/93)
+  documented).
+  Reported by Lindsay Errington, @dlindsaye,
+  https://github.com/thierry-martinez/pyml/issues/93
 
 - #91, #92, #94: Better search heuristics for `python` library.
   Suggested by camlspotter and Et7f3.
   Use of `python-config`.
   Use of `otool -L` instead of `ldd` on Mac OS X.
-  (https://github.com/thierry-martinez/pyml/issues/91
-   https://github.com/thierry-martinez/pyml/issues/92 )
+  https://github.com/thierry-martinez/pyml/issues/91
+  https://github.com/thierry-martinez/pyml/issues/92
+
+- #96: `find` functions (`Py.Object.find`, `Py.Object.find_string`,
+  `Py.Dict.find`, `Py.Dict.find_string`, `Py.Object.find_attr_string`
+  and `Py.Object.find_attr`) now consistently fail with `Not_found`
+  exception, as it is said in the documentation.  Functions
+  `Py.Object.find_err`, `Py.Object.find_string_err`,
+  `Py.Object.find_attr_err`, `Py.Object.find_attr_string_err` have
+  been introduced for cases where keeping the underlying Python
+  exception is preferable.  For instance, `Py.Module.get` is now an
+  alias for `Py.Object.find_attr_string_err` to keep the current
+  behavior of failing with a Python exception.
+  Reported by Jonathan Laurent,
+  https://github.com/thierry-martinez/pyml/issues/96
 
 # 2022-09-05
 

py.ml

@@ -945,6 +945,15 @@ let option result =
   else
     Some result
 
+let check_found result =
+  if result = null then
+    begin
+      check_error ();
+      raise Not_found
+    end
+  else
+    result
+
 let option_of_error result =
   if result = null then
     begin
@@ -1194,7 +1203,7 @@ module Mapping = struct
     option (Pywrappers.pymapping_getitemstring mapping key)
 
   let find_string mapping key =
-    Stdcompat.Option.get (get_item_string mapping key)
+    check_found (Pywrappers.pymapping_getitemstring mapping key)
 
   let find_string_opt = get_item_string
 
@@ -1555,6 +1564,16 @@ end
 
 exception Err of Err.t * string
 
+let attribute_error = "AttributeError"
+
+let check_found_catch error result =
+  try
+    check_found result
+  with E (ty, _)
+  when
+    String.to_string (check_found (Pywrappers.pyobject_getattrstring ty "__name__")) = error ->
+    raise Not_found
+
 module Object = struct
   include Object_
 
@@ -1574,14 +1593,26 @@ module Object = struct
     option (Pywrappers.pyobject_getattr obj attr)
 
   let find_attr_string obj attr =
+    assert_not_null "find_attr_string" obj;
+    check_found_catch attribute_error (Pywrappers.pyobject_getattrstring obj attr)
+
+  let find_attr_string_err obj attr =
     assert_not_null "find_attr_string" obj;
     check_not_null (Pywrappers.pyobject_getattrstring obj attr)
 
   let get_attr_string obj attr =
     assert_not_null "find_attr_string" obj;
     option_of_error (Pywrappers.pyobject_getattrstring obj attr)
 
-  let find_attr obj attr = Stdcompat.Option.get (get_attr obj attr)
+  let find_attr obj attr =
+    assert_not_null "find_attr(!, _)" obj;
+    assert_not_null "find_attr(_, !)" attr;
+    check_found_catch attribute_error (Pywrappers.pyobject_getattr obj attr)
+
+  let find_attr_err obj attr =
+    assert_not_null "find_attr(!, _)" obj;
+    assert_not_null "find_attr(_, !)" attr;
+    check_not_null (Pywrappers.pyobject_getattr obj attr)
 
   let find_attr_opt = get_attr
 
@@ -1590,13 +1621,19 @@ module Object = struct
   let get_item obj key =
     option (Pywrappers.pyobject_getitem obj key)
 
-  let find obj attr = Stdcompat.Option.get (get_item obj attr)
+  let find obj attr =
+    check_found_catch "KeyError" (Pywrappers.pyobject_getitem obj attr)
+
+  let find_err obj attr =
+    check_not_null (Pywrappers.pyobject_getitem obj attr)
 
   let find_opt = get_item
 
   let get_item_string obj key = get_item obj (String.of_string key)
 
-  let find_string obj attr = Stdcompat.Option.get (get_item_string obj attr)
+  let find_string obj key = find obj (String.of_string key)
+
+  let find_string_err obj key = find_err obj (String.of_string key)
 
   let find_string_opt = get_item_string
 
@@ -2221,15 +2258,20 @@ module Dict = struct
     assert_not_null "get_item(_, !)" key;
     option (Pywrappers.pydict_getitem dict key)
 
-  let find dict key = Stdcompat.Option.get (get_item dict key)
+  let find dict key =
+    assert_not_null "get_item(!, _)" dict;
+    assert_not_null "get_item(_, !)" key;
+    check_found (Pywrappers.pydict_getitem dict key)
 
   let find_opt = get_item
 
   let get_item_string dict name =
     assert_not_null "get_item_string" dict;
     option (Pywrappers.pydict_getitemstring dict name)
 
-  let find_string dict key = Stdcompat.Option.get (get_item_string dict key)
+  let find_string dict key =
+    assert_not_null "get_item_string" dict;
+    check_found (Pywrappers.pydict_getitemstring dict key)
 
   let find_string_opt = get_item_string
 
@@ -2581,7 +2623,7 @@ module Module = struct
     assert_not_null "get_name" m;
     check_some (Pywrappers.pymodule_getname m)
 
-  let get = Object.find_attr_string
+  let get = Object.find_attr_string_err
 
   let get_opt = Object.find_attr_string_opt
 

py.mli

@@ -132,6 +132,10 @@ module Object: sig
   (** Equivalent to {!get_attr} but raises a [Not_found] exception in
       case of failure. *)
 
+  val find_attr_err: t -> t -> t
+  (** Equivalent to {!get_attr} but raises a Python exception in
+      case of failure. *)
+
   val find_attr_opt: t -> t -> t option
   (** Alias for {!get_attr}. *)
 
@@ -143,6 +147,10 @@ module Object: sig
   (** Equivalent to {!get_attr_string} but raises a [Not_found] exception in
       case of failure. *)
 
+  val find_attr_string_err: t -> string -> t
+  (** Equivalent to {!get_attr_string} but raises a Python exception in
+      case of failure. *)
+
   val find_attr_string_opt: t -> string -> t option
   (** Alias for {!get_attr_string}. *)
 
@@ -154,6 +162,10 @@ module Object: sig
   (** Equivalent to {!get_item} but raises a [Not_found] exception in
       case of failure. *)
 
+  val find_err: t -> t -> t
+  (** Equivalent to {!get_item} but raises a Python exception in
+      case of failure. *)
+
   val find_opt: t -> t -> t option
   (** Alias for {!get_item}. *)
 
@@ -165,6 +177,10 @@ module Object: sig
   (** Equivalent to {!get_item_string} but raises a [Not_found] exception in
       case of failure. *)
 
+  val find_string_err: t -> string -> t
+  (** Equivalent to {!get_item_string} but raises a Python exception in
+      case of failure. *)
+
   val find_string_opt: t -> string -> t option
   (** Alias for {!get_item_string}. *)
 
@@ -1331,7 +1347,7 @@ module Module: sig
       {{:https://docs.python.org/3/c-api/module.html#c.PyModule_GetName} PyModule_GetName} *)
 
   val get: Object.t -> string -> Object.t
-  (** Equivalent to {!Object.find_attr_string}. *)
+  (** Equivalent to {!Object.find_attr_string_err}. *)
 
   val get_opt: Object.t -> string -> Object.t option
   (** Equivalent to {!Object.find_attr_string_opt}. *)

pyml_tests.ml

@@ -717,6 +717,79 @@ let () =
        assert (Py.Object.get_attr_string bool_ty "dtype" = None);
        Pyml_tests_common.Passed)
 
+let () =
+  Pyml_tests_common.add_test
+    ~title:"Dict.find fails with Not_found"
+    (fun () ->
+       let dict = Py.Dict.create() in
+       try
+         let _ = Py.Dict.find dict Py.Tuple.empty in
+         Pyml_tests_common.Failed "Unexpected found"
+       with Not_found ->
+         Pyml_tests_common.Passed)
+
+let () =
+  Pyml_tests_common.add_test
+    ~title:"Dict.find_string fails with Not_found"
+    (fun () ->
+       let dict = Py.Dict.create() in
+       try
+         let _ = Py.Dict.find_string dict "key" in
+         Pyml_tests_common.Failed "Unexpected found"
+       with Not_found ->
+         Pyml_tests_common.Passed)
+
+let () =
+  Pyml_tests_common.add_test
+    ~title:"Object.find_attr fails with Not_found if key is missing"
+    (fun () ->
+       try
+         let _ =
+           Py.Object.find_attr Py.Tuple.empty (Py.String.of_string "key") in
+         Pyml_tests_common.Failed "Unexpected found"
+       with Not_found ->
+         Pyml_tests_common.Passed)
+
+let () =
+  Pyml_tests_common.add_test
+    ~title:"Object.find_attr fails with Python exception if wrong key type"
+    (fun () ->
+       try
+         let _ = Py.Object.find_attr Py.Tuple.empty Py.Tuple.empty in
+         Pyml_tests_common.Failed "Unexpected found"
+       with Py.E _ ->
+         Pyml_tests_common.Passed)
+
+let () =
+  Pyml_tests_common.add_test
+    ~title:"Object.find_attr_string fails with Not_found if key is missing"
+    (fun () ->
+       try
+         let _ = Py.Object.find_attr_string Py.Tuple.empty "key" in
+         Pyml_tests_common.Failed "Unexpected found"
+       with Not_found ->
+         Pyml_tests_common.Passed)
+
+let () =
+  Pyml_tests_common.add_test
+    ~title:"Object.find fails with Not_found if key is missing"
+    (fun () ->
+       try
+         let _ = Py.Object.find (Py.Dict.create ()) (Py.String.of_string "key") in
+         Pyml_tests_common.Failed "Unexpected found"
+       with Not_found ->
+         Pyml_tests_common.Passed)
+
+let () =
+  Pyml_tests_common.add_test
+    ~title:"Object.find fails with Not_found if wrong key type"
+    (fun () ->
+       try
+         let _ = Py.Object.find Py.Tuple.empty Py.Tuple.empty in
+         Pyml_tests_common.Failed "Unexpected found"
+       with Py.E _ ->
+         Pyml_tests_common.Passed)
+
 let () =
   if not !Sys.interactive then
     Pyml_tests_common.main ()

pyml_tests_common.ml

@@ -22,9 +22,10 @@ let launch_test (title, f) =
        failed := true
     | Disabled reason -> Printf.printf "disabled: %s\n%!" reason
   with
-    Py.E (_, value) ->
+    Py.E (ty, value) ->
     Printf.printf
-      "raised a Python exception: %s\n%!"
+      "raised a Python exception: [%s] %s\n%!"
+      (Py.Object.to_string ty)
       (Py.Object.to_string value);
     failed := true
   | e ->