-
Notifications
You must be signed in to change notification settings - Fork 2
MS-990 Custom model file support #14
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
Conversation
0c144cc to
649442f
Compare
649442f to
bd27df7
Compare
Thinkorswim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Might be useful to separate the files in androidTest into tests and utilities folders to make it a bit cleaner.
| assertArrayEquals(GOOD_FACE_EMBEDDING, resultFloat, 0.1F) | ||
| } | ||
|
|
||
| private fun openTestModelFile(resourceName: String = "edgeface_test"): File { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we extract this logic into a separate utils file? It seems like it could be reusable across future tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed into an extention method of Context and moved to TestContextExt.
| return tempFile | ||
| } | ||
|
|
||
| private fun getFaceEmbeddingFromBitmap(bitmap: Bitmap): FloatArray = simFace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same with this, otherwise not really useful to have it as a separate function.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pretty specific for this test class. It was extracted into a function to make the test case easier to read.
gstrag
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice improvements!
This started as a spike to see if it is possible, but turned into a pretty nice future-proof solution. Here are the main changes:
Benefits of this change: