Merge pull request #94 from thierry-martinez/fix.93.get_attr_string

Fix #93: `get_attr_string` returns `None` when attribute is missing

CHANGES.md

@@ -7,6 +7,14 @@
   (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)
+
+- #94: Better search heuristics for `python` library
+
 # 2022-09-05
 
 - Support for OCaml 5.0

py.ml

@@ -444,19 +444,23 @@ let libpython_from_python_config version_major version_minor =
       Some (concat_library_filenames library_paths library_filenames)
   | _ -> None
 
+let getenv_opt var =
+  try Some (Sys.getenv var)
+  with Not_found -> None
+
 let libpython_from_pythonhome version_major version_minor python_full_path =
   let library_paths =
     match
-      try Some (Sys.getenv "PYTHONHOME")
-      with Not_found ->
+      match getenv_opt "PYTHONHOME" with
+      | Some python_home -> Some (Pyutils.split_left_on_char ':' python_home)
+      | None ->
         match python_full_path with
-          None -> None
         | Some python_full_path -> Some (parent_dir python_full_path)
+        | None -> None
     with
       None -> failwith "Unable to find libpython!"
-    | Some pythonhome ->
-        let prefix = Pyutils.split_left_on_char ':' pythonhome in
-        [Filename.concat prefix "lib"] in
+    | Some dir ->
+        [Filename.concat dir "lib"] in
   let library_filenames =
     List.map
       (fun format -> format version_major version_minor)
@@ -485,9 +489,7 @@ let find_library_path version_major version_minor python_full_path =
           Some (libpython_from_pythonhome version_major version_minor
             python_full_path))));
   ] in
-  match List.find_map (fun f -> f ()) heuristics with
-  | None -> failwith "Cannot find Python library"
-  | Some paths -> paths
+  List.concat (List.map (fun f -> Option.value ~default:[] (f ())) heuristics)
 
 let python_version_from_interpreter interpreter =
   let version_line =
@@ -910,6 +912,15 @@ let option result =
   else
     Some result
 
+let option_of_error result =
+  if result = null then
+    begin
+      let _ = pyerr_fetch_internal () in
+      None
+    end
+  else
+    Some result
+
 let assert_not_null function_name obj =
   if is_null obj then
     invalid_arg (function_name ^ ": unallowed null argument")
@@ -1529,17 +1540,18 @@ module Object = struct
     assert_not_null "get_attr(_, !)" attr;
     option (Pywrappers.pyobject_getattr obj attr)
 
+  let find_attr_string 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 "get_attr_string" obj;
-    option (Pywrappers.pyobject_getattrstring 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_opt = get_attr
 
-  let find_attr_string obj attr =
-    Stdcompat.Option.get (get_attr_string obj attr)
-
   let find_attr_string_opt = get_attr_string
 
   let get_item obj key =

pyml_tests.ml

@@ -709,6 +709,14 @@ let () =
       assert (Py.String.to_string unpickled = "hello");
       Pyml_tests_common.Passed)
 
+let () =
+  Pyml_tests_common.add_test
+    ~title:"get_attr_string"
+    (fun () ->
+       let bool_ty = Py.Object.get_type Py.Bool.t in
+       assert (Py.Object.get_attr_string bool_ty "dtype" = None);
+       Pyml_tests_common.Passed)
+
 let () =
   if not !Sys.interactive then
     Pyml_tests_common.main ()