Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow pathlib.Path values as input to Model construction #54

Merged
merged 1 commit into from
Oct 11, 2023
Merged

Allow pathlib.Path values as input to Model construction #54

merged 1 commit into from
Oct 11, 2023

Conversation

ashay
Copy link
Contributor

@ashay ashay commented Oct 10, 2023

Prior to this patch, only string values could be used to specify the
path to the input ONNX file. However, several Python programs use
values of type pathlib.Path, which causes the model creation to
silently fail in onnx-tool. This patch fixes the problem by allowing
both string paths or pathlib.Path paths.

Credit to @xich for discovering the problem!

Prior to this patch, only string values could be used to specify the
path to the input ONNX file.  However, several Python programs use
values of type `pathlib.Path`, which causes the model creation to
silently fail in onnx-tool.  This patch fixes the problem by allowing
both string paths or `pathlib.Path` paths.

Credit to @xich for discovering the problem!
@ashay
Copy link
Contributor Author

ashay commented Oct 10, 2023

Here is a script to reproduce the error:

import onnx_tool
from pathlib import Path

model = onnx_tool.Model(Path("/tmp/model.onnx"))
print(model.graph)  # AttributeError: 'Model' object has no attribute 'graph'

Copy link
Owner

@ThanatosShinji ThanatosShinji left a comment

Choose a reason for hiding this comment

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

LGTM

@ThanatosShinji ThanatosShinji merged commit dd4373b into ThanatosShinji:main Oct 11, 2023
@ashay ashay deleted the ashay/pathlib-for-model-construction branch October 11, 2023 14:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants