Skip to content

Conversation

zhenhuaw-me
Copy link
Member

Respect the proto def. Close #5552.

@zhenhuaw-me zhenhuaw-me requested a review from a team as a code owner September 4, 2023 08:39
@zhenhuaw-me zhenhuaw-me self-assigned this Sep 4, 2023
Respect the proto def. Close onnx#5552.

Signed-off-by: Zhenhua Wang <zhenhuaw@nvidia.com>
@zhenhuaw-me zhenhuaw-me requested a review from xadupre September 4, 2023 14:15
@jcwchen jcwchen added the module: utility Helper modules label Sep 4, 2023
Copy link
Member

@jcwchen jcwchen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR! Only allowing relative path makes sense to me. Might be worthy to mention this limitation in the document for external data as well: https://github.com/onnx/onnx/blob/main/docs/PythonAPIOverview.md#loading-an-onnx-model-with-external-data.

Another document actually has mentioned the location should be relative path already: https://github.com/onnx/onnx/blob/main/docs/ExternalData.md#external_data-field.

@xadupre xadupre added this pull request to the merge queue Sep 4, 2023
Merged via the queue into onnx:main with commit 3a76db8 Sep 4, 2023
corwinjoy pushed a commit to corwinjoy/onnx that referenced this pull request Sep 5, 2023
Respect the proto def. Close onnx#5552.

Signed-off-by: Zhenhua Wang <zhenhuaw@nvidia.com>
Signed-off-by: Corwin Joy <corwinjoy@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: utility Helper modules
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Absolute path should be rejected in onnx.save_model(localtion=)
3 participants