Skip to content
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

feat: add a basic example of text detection #999

Merged
merged 1 commit into from
Jul 27, 2022

Conversation

ianardee
Copy link
Contributor

A script to extract text from files.

@codecov
Copy link

codecov bot commented Jul 27, 2022

Codecov Report

Merging #999 (99bbc86) into main (23d1a1e) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #999      +/-   ##
==========================================
- Coverage   94.85%   94.83%   -0.02%     
==========================================
  Files         134      134              
  Lines        5558     5558              
==========================================
- Hits         5272     5271       -1     
- Misses        286      287       +1     
Flag Coverage Δ
unittests 94.83% <ø> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
doctr/transforms/functional/base.py 94.20% <0.00%> (-1.45%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us.

@felixdittrich92 felixdittrich92 added this to the 0.6.0 milestone Jul 27, 2022
@felixdittrich92 felixdittrich92 added type: enhancement Improvement ext: scripts Related to scripts folder topic: text detection Related to the task of text detection labels Jul 27, 2022
@felixdittrich92
Copy link
Contributor

felixdittrich92 commented Jul 27, 2022

Hi @ianardee 👋,
thanks for the PR :)
Some minor things:

  • could we add also the xml output ?
  • missing (minor/lightweight) CI test
  • maybe rename to extract_text otherwise as a user i would expect to get only the box coords wdyt ?
  • missing .cuda() if backend is pytorch and gpu available (silent move)
  • pass can be removed
  • for string export you should use the .render() method

Sry for the lazy review i have tried to do it from my mobile phone 😅

Copy link
Collaborator

@charlesmindee charlesmindee left a comment

Choose a reason for hiding this comment

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

LGTM, thanks

@charlesmindee charlesmindee merged commit ada1bf2 into main Jul 27, 2022
@charlesmindee charlesmindee deleted the add-text-detect-script branch July 27, 2022 16:09
@felixdittrich92
Copy link
Contributor

felixdittrich92 commented Jul 27, 2022

@charlesmindee @frgfm I think there are some missing things (mentioned above) wdyt ? 🤔

@ianardee
Copy link
Contributor Author

@felixdittrich92 I'll make another PR to answer some of your concerns.

@felixdittrich92
Copy link
Contributor

@ianardee top 👍

Copy link
Collaborator

@frgfm frgfm left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the script!

I added some comments if you do a follow-up PR :)

@@ -0,0 +1,118 @@
# Copyright (C) 2021-2022, Mindee.
Copy link
Collaborator

Choose a reason for hiding this comment

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

# Copyright (C) 2022, Mindee.

This script didn't exist in 2021 :)

Comment on lines +48 to +49
for word in line["words"]:
out_txt += word["value"] + " "
Copy link
Collaborator

Choose a reason for hiding this comment

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

Perhaps

out_txt += " ".join(word["value"] for word in line["words"])

or wrapping more nested loops inside a list comprehensions 🤷‍♂️

Copy link
Contributor

Choose a reason for hiding this comment

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

out.render() ?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh yeah actually, I had forgotten about this haha

model = ocr_predictor(args.detection, args.recognition, pretrained=True)
path = Path(args.path)
if path.is_dir():
allowed = (".pdf", ".jpeg", ".jpg", ".png", ".tif", ".tiff", ".bmp")
Copy link
Collaborator

Choose a reason for hiding this comment

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

perhaps we should move this at the top of the files with other constants?

fh.write(out_str)
else:
out_str = _process_file(model, path, args.format)
print(out_str)
Copy link
Collaborator

Choose a reason for hiding this comment

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

in one case, we dump the string into a file and in the other we print it?

@felixdittrich92
Copy link
Contributor

Hi @ianardee 👋 , any updates about the refactor PR ? :)

@felixdittrich92 felixdittrich92 mentioned this pull request Sep 26, 2022
85 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ext: scripts Related to scripts folder topic: text detection Related to the task of text detection type: enhancement Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants