Repository: Lightning-AI/pytorch-lightning
Pull request: Re-enable passing BytesIO as path in `.to_onnx()`
## What does this PR do?

<!--
Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.

If we didn't discuss your PR in Github issues there's a high chance it will not be merged.

The following links the related issue to the PR (https://docs.github.com/en/free-pro-team@latest/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)
-->

Fixes #20171

In `to_onnx`, only convert `file_path` to string when its a `Path` instance, and keep `io.BytesIO` intact and pass it to `torch.export.onnx`.

<!-- Does your PR introduce any breaking changes? If yes, please list them. -->

<details>
  <summary><b>Before submitting</b></summary>

- Was this **discussed/agreed** via a GitHub issue? (not for typos and docs)
- [ ] Did you read the [contributor guideline](https://github.com/Lightning-AI/lightning/blob/master/.github/CONTRIBUTING.md), **Pull Request** section?
- [ ] Did you make sure your **PR does only one thing**, instead of bundling different changes together?
- Did you make sure to **update the documentation** with your changes? (if necessary)
- Did you write any **new necessary tests**? (not for typos and docs)
- [ ] Did you verify new and **existing tests pass** locally with your changes?
- Did you list all the **breaking changes** introduced by this pull request?
- Did you **update the CHANGELOG**? (not for typos, docs, test updates, or minor internal changes/refactors)

<!-- In the CHANGELOG, separate each item in the unreleased section by a blank line to reduce collisions -->

</details>

## PR review

Anyone in the community is welcome to review the PR.
Before you start reviewing, make sure you have read the [review guidelines](https://github.com/Lightning-AI/lightning/wiki/Review-guidelines). In short, see the following bullet-list:

<details>
  <summary>Reviewer checklist</summary>

- [ ] Is this pull request ready for review? (if not, please submit in draft mode)
- [ ] Check that all items from **Before submitting** are resolved
- [ ] Make sure the title is self-explanatory and the description concisely explains the PR
- [ ] Add labels and milestones (and optionally projects) to the PR so it can be classified

</details>

<!--

Did you have fun?

Make sure you had fun coding 🙃

-->


<!-- readthedocs-preview pytorch-lightning start -->
----
📚 Documentation preview 📚: https://pytorch-lightning--20172.org.readthedocs.build/en/20172/

<!-- readthedocs-preview pytorch-lightning end -->

Problem statement:
Inconsistent input io type between `to_onnx`  and `torch.onnx.export`.
### Bug description

Currently the filetype supported in `torch.onnx.export` includes `io.BytesIO`, whereas in `lightning`, it only supports `str` and `PathLike` object. Before lightning `2.3.3` , passing a `BytesIO` wouldn't be a problem since `to_onnx` did not do anything with `file_path`, but since this  version, it changed by passing `str(file_path)`, which will cause problems when passing an `BytesIO` instance.

### What version are you seeing the problem on?

v2.3

### How to reproduce the bug

```python
from io import BytesIO
model = LightningModel()

onnx_io = BytesIO()
model.to_onnx(onnx_io)
```


### Error messages and logs

```
OSError: [Errno 22] Invalid argument: '<_io.BytesIO object at 0x000002487558E3B0>'
```


### Environment

<details>
  <summary>Current environment</summary>

```
#- PyTorch Lightning Version (e.g., 2.4.0):
#- PyTorch Version (e.g., 2.4):
#- Python version (e.g., 3.12):
#- OS (e.g., Linux):
#- CUDA/cuDNN version:
#- GPU models and configuration:
#- How you installed Lightning(`conda`, `pip`, source):
```

</details>


### More info

_No response_

Hints from issue discussion:

Commit message: fix: change path to str if `file_path` is Path instance. (fix #20171)

Patch to review:
diff --git a/src/lightning/pytorch/core/module.py b/src/lightning/pytorch/core/module.py
index 647f6e6e41af7..ef006b676e55a 100644
--- a/src/lightning/pytorch/core/module.py
+++ b/src/lightning/pytorch/core/module.py
@@ -1364,7 +1364,7 @@ def _verify_is_manual_optimization(self, fn_name: str) -> None:
             )
 
     @torch.no_grad()
-    def to_onnx(self, file_path: Union[str, Path], input_sample: Optional[Any] = None, **kwargs: Any) -> None:
+    def to_onnx(self, file_path: Union[str, Path, IO], input_sample: Optional[Any] = None, **kwargs: Any) -> None:
         """Saves the model in ONNX format.
 
         Args:
@@ -1403,7 +1403,9 @@ def forward(self, x):
         input_sample = self._on_before_batch_transfer(input_sample)
         input_sample = self._apply_batch_transfer_handler(input_sample)
 
-        torch.onnx.export(self, input_sample, str(file_path), **kwargs)
+        if isinstance(file_path, Path):
+            file_path = file_path.as_posix()
+        torch.onnx.export(self, input_sample, file_path, **kwargs)
         self.train(mode)
 
     @torch.no_grad()


Code review comments:
diff_hunk: @@ -1403,7 +1403,9 @@ def forward(self, x):
         input_sample = self._on_before_batch_transfer(input_sample)
         input_sample = self._apply_batch_transfer_handler(input_sample)
 
-        torch.onnx.export(self, input_sample, str(file_path), **kwargs)
+        if isinstance(file_path, Path):
+            file_path = file_path.as_posix()
path: src/lightning/pytorch/core/module.py
text: @user1:
I think we should keep what #19727 did. We try to avoid making assumptions on posix and keep handling paths OS-independent whenever possible.
```suggestion
        file_path = str(file_path) if isinstance(file_path, Path) else file_path
```

@author:
Hi @user1 
this is fixed in the new commits. Thanks.