Repository: onnx/onnx
Pull request: Add `os.PathLike` when filepath as `str` is expected
### Description

closes #5182 by adding `os.PathLike` to type unions and using `os.fspath` at some points.

### Motivation and Context

#### Context
`onnx` file handling accepts only `IO[bytes] | str`. If passing Python `pathlib.Path`, static type checkers such as `pyright` and `pylance` emit errors which is incorrect.

```python
import onnx
import pathlib

model = onnx.load(pathlib.Path("model.onnx"))
```
Causes `PyRight` error.

#### Motivation

1. Using `pathlib` to handle paths is considered best practice by many.
2. Would remove unnecessary object transformations to satisfy type checkers
3. Reduces # type: ignore usage in user code base.
These, in general, should benefit general code quality.

<!-- - If it fixes an open issue, please link to the issue here. -->
Initial issue discussion was with

[justinchuby](https://github.com/justinchuby)

This is my first time contributing to anything public. Changes are very minor and any feedback is welcome.

Problem statement:
[Feature request] Add possibility to pass `pathlib.Path` where filepath is expected
### System information

 name         : onnx                         
 version      : 1.13.1                       
 description  : Open Neural Network Exchange 

dependencies
 - numpy >=1.16.6
 - protobuf >=3.20.2,<4
 - typing-extensions >=3.6.2.1

required by
 - onnxconverter-common *
 - skl2onnx >=1.2.1

### What is the problem that this feature solves?

`onnx` file handling accepts only `IO[bytes] | str`. If passing Python `pathlib.Path`, static type checkers such as `pyright` and `pylance` emit errors which is incorrect.

```python
import onnx
import pathlib

model = onnx.load(pathlib.Path("model.onnx"))
```
Causes `PyRight` error

```
Argument of type "Path" cannot be assigned to parameter "f" of type "IO[bytes] | str" in function "load_model"
    Type "Path" cannot be assigned to type "IO[bytes] | str"
      "Path" is incompatible with "IO[bytes]"
      "Path" is incompatible with "str" (reportGeneralTypeIssues)
```
Commonly, usage `pathlib` is considered preferential to `str`, as it reduces the risk of manual error.

### Alternatives considered


One can avert this error by

```python
import pathlib
import onnx
import typing
modelpath = pathlib.Path("model.onnx")
model = onnx.load(str(modelpath))
# or
with open(modelpath, "rb") as fobj:
    model = onnx.load(fobj)
# or 
model = onnx.load(typing.cast(str, modelpath))
# or doing over-engineered staff with pathlib.Path
```

Or one can use manual path handling with `str`, which as previously mentioned, is often considered non-optimal.

### Describe the feature

1. Using `pathlib` to handle paths is considered best practice by many.
2. Would remove unnecessary object transformations to satisfy type checkers 
3. Reduces `# type: ignore` usage in user code base. 

These, in general, should benefit general code quality.

### Will this influence the current api (Y/N)?

No. The current code that expects path should work when passed a `pathlib.Path`. This feature should only affect the type hints. 

### Feature Area

best practices, type hinting

### Are you willing to contribute it (Y/N)

Yes

### Notes

This change is very minor and could probably be done in a few hours by a "replace all" `Union[IO[bytes], str`] -> `Union[IO[bytes], str, Path]`.

Hints from issue discussion:
We can use `os.fspath` for this when the input is `os.PathLike`. Note we should not use `str`. Contribution is appreciated! I would consider:

1. Add `os.PathLike` to the type union.
2. Use `os.fspath` in cases we need to take the path as a raw string (unlikely needed because we are using open() for reading files. We can also move to use pathlib internally)

This will also need to be coordinated with #5112 and #5183

Commit message: Merge branch 'main' into main

Patch to review:
diff --git a/.gitignore b/.gitignore
index 0391f113905..84df2918ed3 100644
--- a/.gitignore
+++ b/.gitignore
@@ -110,3 +110,6 @@ docs/docsgen/source/onnx-api/modules/
 docs/docsgen/source/operators/
 docs/docsgen/**/*.onnx
 docs/docsgen/**/*.pb
+
+# PyEnv files
+.python-version
\ No newline at end of file
diff --git a/onnx/__init__.py b/onnx/__init__.py
index d0614416613..5a8d726f137 100644
--- a/onnx/__init__.py
+++ b/onnx/__init__.py
@@ -135,25 +135,27 @@
 _SupportedFormat = Union[Literal["protobuf", "textproto"], str]
 
 
-def _load_bytes(f: IO[bytes] | str) -> bytes:
+def _load_bytes(f: IO[bytes] | str | os.PathLike) -> bytes:
     if hasattr(f, "read") and callable(typing.cast(IO[bytes], f).read):
         content = typing.cast(IO[bytes], f).read()
     else:
-        with open(typing.cast(str, f), "rb") as readable:
+        f = typing.cast(Union[str, os.PathLike], f)
+        with open(f, "rb") as readable:
             content = readable.read()
     return content
 
 
-def _save_bytes(content: bytes, f: IO[bytes] | str) -> None:
+def _save_bytes(content: bytes, f: IO[bytes] | str | os.PathLike) -> None:
     if hasattr(f, "write") and callable(typing.cast(IO[bytes], f).write):
         typing.cast(IO[bytes], f).write(content)
     else:
-        with open(typing.cast(str, f), "wb") as writable:
+        f = typing.cast(Union[str, os.PathLike], f)
+        with open(f, "wb") as writable:
             writable.write(content)
 
 
-def _get_file_path(f: IO[bytes] | str) -> str | None:
-    if isinstance(f, str):
+def _get_file_path(f: IO[bytes] | str | os.PathLike) -> str | None:
+    if isinstance(f, (str, os.PathLike)):
         return os.path.abspath(f)
     if hasattr(f, "name"):
         return os.path.abspath(f.name)
@@ -161,14 +163,14 @@ def _get_file_path(f: IO[bytes] | str) -> str | None:
 
 
 def load_model(
-    f: IO[bytes] | str,
+    f: IO[bytes] | str | os.PathLike,
     format: _SupportedFormat = "protobuf",  # pylint: disable=redefined-builtin
     load_external_data: bool = True,
 ) -> ModelProto:
     """Loads a serialized ModelProto into memory.
 
     Args:
-        f: can be a file-like object (has "read" function) or a string containing a file name
+        f: can be a file-like object (has "read" function) or a string/PathLike containing a file name
         format: The serialization format. Default is "protobuf". The encoding is
             assumed to be "utf-8" when the format is a text format.
         load_external_data: Whether to load the external data.
@@ -191,13 +193,13 @@ def load_model(
 
 
 def load_tensor(
-    f: IO[bytes] | str,
+    f: IO[bytes] | str | os.PathLike,
     format: _SupportedFormat = "protobuf",  # pylint: disable=redefined-builtin
 ) -> TensorProto:
     """Loads a serialized TensorProto into memory.
 
     Args:
-        f: can be a file-like object (has "read" function) or a string containing a file name
+        f: can be a file-like object (has "read" function) or a string/PathLike containing a file name
         format: The serialization format. Default is "protobuf". The encoding is
             assumed to be "utf-8" when the format is a text format.
 
@@ -243,7 +245,7 @@ def load_tensor_from_string(
 
 def save_model(
     proto: ModelProto | bytes,
-    f: IO[bytes] | str,
+    f: IO[bytes] | str | os.PathLike,
     format: _SupportedFormat = "protobuf",  # pylint: disable=redefined-builtin
     *,
     save_as_external_data: bool = False,
@@ -257,7 +259,8 @@ def save_model(
 
     Args:
         proto: should be a in-memory ModelProto
-        f: can be a file-like object (has "write" function) or a string containing a file name
+        f: can be a file-like object (has "write" function) or a string containing
+        a file name or a pathlike object
         format: The serialization format. Default is "protobuf". The encoding is
             assumed to be "utf-8" when the format is a text format.
         save_as_external_data: If true, save tensors to external file(s).
@@ -295,7 +298,7 @@ def save_model(
 
 def save_tensor(
     proto: TensorProto,
-    f: IO[bytes] | str,
+    f: IO[bytes] | str | os.PathLike,
     format: _SupportedFormat = "protobuf",  # pylint: disable=redefined-builtin
 ) -> None:
     """
@@ -303,7 +306,8 @@ def save_tensor(
 
     Args:
         proto: should be a in-memory TensorProto
-        f: can be a file-like object (has "write" function) or a string containing a file name
+        f: can be a file-like object (has "write" function) or a string
+        containing a file name format  or a pathlike object: for future use
         format: The serialization format. Default is "protobuf". The encoding is
             assumed to be "utf-8" when the format is a text format.
     """
diff --git a/onnx/checker.py b/onnx/checker.py
index 60dfbbb85d8..826a3d99168 100644
--- a/onnx/checker.py
+++ b/onnx/checker.py
@@ -6,10 +6,11 @@
 This implements graphalities that allows us to check whether a serialized
 proto is legal.
 """
-
+from __future__ import annotations
 import functools
+import os
 import sys
-from typing import Any, Callable, Optional, Type, TypeVar, Union, cast
+from typing import Any, Callable, Optional, Type, TypeVar, cast
 
 from google.protobuf.message import Message
 
@@ -113,16 +114,18 @@ def check_sparse_tensor(
     C.check_sparse_tensor(sparse.SerializeToString(), ctx)
 
 
-def check_model(model: Union[ModelProto, str, bytes], full_check: bool = False) -> None:
+def check_model(
+    model: ModelProto | str | bytes | os.PathLike, full_check: bool = False
+) -> None:
     """Check the consistency of a model. An exception is raised if the test fails.
 
     Arguments:
-        model (ModelProto): model to check
+        model (ModelProto | str | bytes | os.PathLike): model to check
         full_check (bool): if True, the function checks shapes can be inferred
     """
     # If model is a path instead of ModelProto
-    if isinstance(model, str):
-        C.check_model_path(model, full_check)
+    if isinstance(model, (str, os.PathLike)):
+        C.check_model_path(os.fspath(model), full_check)
     else:
         protobuf_string = (
             model if isinstance(model, bytes) else model.SerializeToString()
diff --git a/onnx/test/basic_test.py b/onnx/test/basic_test.py
index 41c0d55e105..4f5654e7bd2 100644
--- a/onnx/test/basic_test.py
+++ b/onnx/test/basic_test.py
@@ -4,6 +4,7 @@
 
 import io
 import os
+import pathlib
 import tempfile
 import unittest
 from typing import Literal
@@ -64,6 +65,14 @@ def test_save_and_load_model_when_input_is_file_name(self) -> None:
             loaded_proto = onnx.load_model(model_path, format=self.format)
             self.assertEqual(proto, loaded_proto)
 
+    def test_save_and_load_model_when_input_is_pathlike(self) -> None:
+        proto = _simple_model()
+        with tempfile.TemporaryDirectory() as temp_dir:
+            model_path = pathlib.Path(temp_dir, "model.onnx")
+            onnx.save_model(proto, model_path, format=self.format)
+            loaded_proto = onnx.load_model(model_path, format=self.format)
+            self.assertEqual(proto, loaded_proto)
+
     def test_load_tensor_when_input_is_bytes(self) -> None:
         proto = _simple_tensor()
         proto_string = serialization.registry.get(self.format).serialize_proto(proto)
@@ -87,6 +96,15 @@ def test_save_and_load_tensor_when_input_is_file_name(self) -> None:
             loaded_proto = onnx.load_tensor(model_path, format=self.format)
             self.assertEqual(proto, loaded_proto)
 
+    def test_save_and_load_tensor_when_input_is_pathlike(self) -> None:
+        # Test if input is a file name
+        proto = _simple_tensor()
+        with tempfile.TemporaryDirectory() as temp_dir:
+            model_path = pathlib.Path(temp_dir, "model.onnx")
+            onnx.save_tensor(proto, model_path, format=self.format)
+            loaded_proto = onnx.load_tensor(model_path, format=self.format)
+            self.assertEqual(proto, loaded_proto)
+
 
 class TestBasicFunctions(unittest.TestCase):
     def test_protos_exist(self) -> None:
diff --git a/onnx/test/test_external_data.py b/onnx/test/test_external_data.py
index 596e65af724..f05c4cdab13 100644
--- a/onnx/test/test_external_data.py
+++ b/onnx/test/test_external_data.py
@@ -4,6 +4,7 @@
 from __future__ import annotations
 
 import os
+import pathlib
 import tempfile
 import unittest
 import uuid
@@ -524,17 +525,18 @@ def setUp(self) -> None:
             tempfile.TemporaryDirectory()  # pylint: disable=consider-using-with
         )
         self.temp_dir: str = self._temp_dir_obj.name
-        self.model_file_path: str = os.path.join(self.temp_dir, "model.onnx")
+        self._model_file_path: str = os.path.join(self.temp_dir, "model.onnx")
         self.large_data = np.random.rand(10, 60, 100).astype(np.float32)
         self.small_data = (200, 300)
         self.model = self.create_test_model()
 
+    @property
+    def model_file_path(self):
+        return self._model_file_path
+
     def tearDown(self) -> None:
         self._temp_dir_obj.cleanup()
 
-    def get_temp_model_filename(self) -> str:
-        return os.path.join(self.temp_dir, str(uuid.uuid4()) + ".onnx")
-
     def create_test_model(self) -> ModelProto:
         X = helper.make_tensor_value_info("X", TensorProto.FLOAT, self.large_data.shape)
         input_init = helper.make_tensor(
@@ -738,5 +740,16 @@ def test_check_model_absolute(self) -> None:
             checker.check_model(self.model_filename)
 
 
+class TestSaveAllTensorsAsExternalDataWithPath(TestSaveAllTensorsAsExternalData):
+    def get_temp_model_filename(self) -> pathlib.Path:
+        return pathlib.Path(super().get_temp_model_filename())
+
+
+class TestExternalDataToArrayWithPath(TestExternalDataToArray):
+    @property
+    def model_file_path(self) -> pathlib.Path:
+        return pathlib.Path(self._model_file_path)
+
+
 if __name__ == "__main__":
     unittest.main()
diff --git a/onnx/utils.py b/onnx/utils.py
index 0e8bdd9b774..b7d571a6fe2 100644
--- a/onnx/utils.py
+++ b/onnx/utils.py
@@ -176,8 +176,8 @@ def extract_model(
 
 
 def extract_model(
-    input_path: str,
-    output_path: str,
+    input_path: str | os.PathLike,
+    output_path: str | os.PathLike,
     input_names: List[str],
     output_names: List[str],
     check_model: bool = True,
@@ -191,8 +191,8 @@ def extract_model(
     subgraph that is connected to the _main graph_ as attributes of these operators.
 
     Arguments:
-        input_path (string): The path to original ONNX model.
-        output_path (string): The path to save the extracted ONNX model.
+        input_path (str | os.PathLike): The path to original ONNX model.
+        output_path (str | os.PathLike): The path to save the extracted ONNX model.
         input_names (list of string): The names of the input tensors that to be extracted.
         output_names (list of string): The names of the output tensors that to be extracted.
         check_model (bool): Whether to run model checker on the extracted model.


Code review comments:
path: onnx/utils.py
text: @user1:
the future import is needed in this file as well

diff_hunk: @@ -295,15 +298,16 @@ def save_model(
 
 def save_tensor(
     proto: TensorProto,
-    f: IO[bytes] | str,
+    f: IO[bytes] | str | os.PathLike,
     format: _SupportedFormat = "protobuf",  # pylint: disable=redefined-builtin
 ) -> None:
     """
     Saves the TensorProto to the specified path.
 
     Args:
         proto: should be a in-memory TensorProto
-        f: can be a file-like object (has "write" function) or a string containing a file name
+        f: can be a file-like object (has "write" function) or a string
+        containing a file name format  or a pathlike object: for future use
path: onnx/__init__.py
text: @user1:
```suggestion
        containing a file name or a pathlike object.
```

here as well