-
Notifications
You must be signed in to change notification settings - Fork 428
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
[references] TF / PT crop & document orientation classifier train scripts #1432
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1432 +/- ##
==========================================
+ Coverage 95.76% 95.84% +0.07%
==========================================
Files 155 162 +7
Lines 6941 7095 +154
==========================================
+ Hits 6647 6800 +153
- Misses 294 295 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
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.
Code seems good 👍
@odulcy-mindee So this part could be merged into 0.8.0 and the rest of this task is for 0.9.0 :) |
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.
Are you fine with the additional OrientationDataset if was thinking on putting it into references/classification/utils but i think the dataset folder is a better fit 😅
@felixdittrich92 yeah, that makes more sense 👍
I added one more suggestion.
For another PR: is it possible to have only one train_{framework}.py
for both purposes ? Otherwise, maybe it makes more sense to rename the other scripts train_{framework}_character.py
or something else ?
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.
Thank you @felixdittrich92 ! 🚀
I totally missed this 😅 |
Ok, we can do that in another PR |
This PR:
Any feedback is welcome :)
Dummy runs (mobilenet_v3_small)