Skip to content

Conversation

TomWildenhain-Microsoft
Copy link
Collaborator

Signed-off-by: Tom Wildenhain [email protected]

@lgtm-com
Copy link

lgtm-com bot commented Jan 13, 2021

This pull request introduces 13 alerts when merging 8cce0ba into 99eb959 - view on LGTM.com

new alerts:

  • 7 for Unused import
  • 3 for Unused local variable
  • 3 for Redundant assignment

@lgtm-com
Copy link

lgtm-com bot commented Jan 13, 2021

This pull request introduces 1 alert when merging d46fb97 into 99eb959 - view on LGTM.com

new alerts:

  • 1 for Unused local variable

@TomWildenhain-Microsoft TomWildenhain-Microsoft force-pushed the tom/tflite_handlers branch 2 times, most recently from efa257d to 478458c Compare January 13, 2021 01:39
Signed-off-by: Tom Wildenhain <[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.

lgtm but we should move all of tflite code under tf2onnx

@@ -0,0 +1,11 @@
# Copyright (c) Microsoft Corporation. All rights reserved.
# Licensed under the MIT license.
"""tf2onnx.tflite_handlers module"""
Copy link
Contributor

Choose a reason for hiding this comment

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

I think all of the tflite code should not start at the root dir and be under tf2onnx.
The structure at the root is something like src, tests, tools and in our case src=tf2onnx and all tflite code should go there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

When flatbuffer generates the interface scripts, they all import each other with lines like from tflite.OperatorCode import OperatorCode and will fail if tflite isn't in the root, but I can find/replace them to fix this.

@TomWildenhain-Microsoft TomWildenhain-Microsoft merged commit c4a2142 into master Jan 15, 2021
@TomWildenhain-Microsoft TomWildenhain-Microsoft deleted the tom/tflite_handlers branch January 15, 2021 01:33
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