Skip to content

Conversation

xadupre
Copy link
Collaborator

@xadupre xadupre commented Jun 28, 2021

No description provided.

Signed-off-by: xavier dupré <[email protected]>
@lgtm-com
Copy link

lgtm-com bot commented Jun 28, 2021

This pull request introduces 2 alerts when merging e04e29d into d4be5af - view on LGTM.com

new alerts:

  • 2 for Unused import

sdpython added 2 commits June 29, 2021 12:50
Signed-off-by: xavier dupré <[email protected]>
Signed-off-by: xavier dupré <[email protected]>
Copy link
Contributor

@guschmue guschmue left a comment

Choose a reason for hiding this comment

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

Not sure if it belongs in examples, maybe consider moving it into a subdir under tests.
There is also tests/run_pretrained_models.py that does similar things ... you could for example add a source 'tfhub' there.

@xadupre
Copy link
Collaborator Author

xadupre commented Jun 29, 2021

Not sure if it belongs in examples, maybe consider moving it into a subdir under tests.
There is also tests/run_pretrained_models.py that does similar things ... you could for example add a source 'tfhub' there.

So I rename example/benchmark into tests/tfhub?

@guschmue
Copy link
Contributor

yes, something like that. If you can make it fairly generic, run_pretrained_models.py would work good I think.

@xadupre
Copy link
Collaborator Author

xadupre commented Jun 29, 2021

I hesitated. I wanted to create something people could copy paste and reuse. run_pretrained_models is very long.

@guschmue
Copy link
Contributor

I did the same for tests/huggingface.py but there some other team wanted to run it outside tf2onnx :(
With tfhub I fear it might not be generic enough ... sometimes one needs to do extra stuff to make it work which might not work well with run_pretrained_models.

Signed-off-by: xavier dupré <[email protected]>
@xadupre xadupre merged commit de67f20 into onnx:master Jun 30, 2021
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.

3 participants