-
Notifications
You must be signed in to change notification settings - Fork 212
Make dependency of vissl optional #1276
Make dependency of vissl optional #1276
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1276 +/- ##
=======================================
Coverage 91.55% 91.56%
=======================================
Files 284 285 +1
Lines 12729 12774 +45
=======================================
+ Hits 11654 11696 +42
- Misses 1075 1078 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
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.
Awesome 😃 Just a couple of minor comments
for more information, see https://pre-commit.ci
Co-authored-by: Ethan Harris <ethanwharris@gmail.com>
Co-authored-by: Ethan Harris <ethanwharris@gmail.com>
Co-authored-by: Ethan Harris <ethanwharris@gmail.com>
Co-authored-by: Ethan Harris <ethanwharris@gmail.com>
This reverts commit 9a1839a.
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
e8f0a40
to
705e8f2
Compare
This PR is ready for review. Please review it. I'm sorry for making a messy commit log. I struggled to solve CI issues. |
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.
LGTM 😃 @ar90n Fine to ignore the deep source issues, we plan to get rid of it. No worries about the commit history! That's why we always squash and merge 😆
@ethanwharris |
@ar90n No worries, thanks for your work on this!! Very glad to see it all working smoothly 😃 |
What does this PR do?
This PR is motivated to achieve the second change in this comment. And this PR contains the following modifications.
training_strategy
,head
,pretraining_transform
in the arguments of__init__
of ImageEmbedder optionalDefaultAdapter
to the Embedding task which is inspired by the Image Classification task's one.Part of #1031
This PR is based on #1271. Because its target branch was wrong.
I'm sorry for my wrong operations. I misunderstood that it would be okay to change its target branch.
Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃