This repository has been archived by the owner on Dec 21, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Feature extraction #3210
Feature extraction #3210
Changes from 43 commits
daa5b4a
63736cf
37d5ccd
67216b5
d891038
18d5b70
40ec7a5
d51b193
ea237b5
f1ea4ac
8e45833
e14a97b
7dc12a1
8cc488d
62aa5e2
e0a301a
1aa4dc1
4bfedd9
e7195d6
c874c97
049c1e5
c02ac72
26a9e61
80dd166
b563393
c4b4ada
b35b2e7
c4798d3
dd0c184
faaf5f2
6d308e8
ab26484
4ff6d46
0efc31b
ca49ab2
b1c27ba
3b4455d
61ac0c2
fd0cbd5
199e1ce
3b97d6f
6a29bc5
0c5ff0c
df86dbd
fd01bcd
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I don't think this right. If the class name doesn't end in
WithDeepFeature
, then I think it should befeature="awesome_image"
. This code is only callingget_deep_features
whenself.feature != "awesome_image"
.I think you need to make this change to all of your test classes.
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 the deep feature column name in test data to the one suggested by you and made all the requested changes.
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.
I think you misunderstood my previous comment. I was not suggesting you need to change the name of the feature. I was basically just trying to say that it's important for the name of the class to represent what that class is actually testing.
For example, if the name of the class is
ImageClassifierResnetTestWithDeepFeatures
, then it should befeature="resnet-50_WithDeepFeature"
orfeature="resnet-50_deep_features",
but notfeature="awesome_image",
. Similarly, if the name of the class isVisionFeaturePrintSceneTest
then it should not be creating the model from deep features, so it should befeature="awesome_image",
notfeature="VisionFeaturePrint_Scene_WithDeepFeature"
.Does that make sense? Let me know if you have any questions.
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.
Oh yeah, you are right I did not see it there. It was correct before but during one of my recent pushes I changed it to test something and forgot to revert it back. My bad, will change it and push it.
And about changing the name of feature columns in the test data in
test_image_classifier.py
andtest_image_similarity.py
to have a suffix_WithDeepFeature
instead of_deep_features
makes more sense to me. So I am keeping that change as it is.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 "fwature" be "function"? We try to maintain a 80 or 100 character length line limit. I would be nice if the text wrapped to the next line after 80 or 100 characters.
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 comment