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

Misc fixed to Streamlit demo #492

Merged
merged 5 commits into from
Sep 28, 2021
Merged

Misc fixed to Streamlit demo #492

merged 5 commits into from
Sep 28, 2021

Conversation

osanseviero
Copy link
Contributor

@osanseviero osanseviero commented Sep 27, 2021

Hello!

I built a Space demo hosting your Streamlit example (link at https://huggingface.co/spaces/osanseviero/doctr). It works great and is much faster than I expected.

I did face some issues during the development. I hope this PR fixes them:

  1. You suggest to install doctr in the requirements but I think that's actually an unrelated package, it should be python-doctr.
  2. But then out.pages[0].synthesize() threw an error saying Page class has no synthesize method. This was introduced in feat: Improved font resolution and page synthesis #472 so I don't think it's in the recent release.

Given 1 and 2, and that #472 will probably be soon in a release, maybe we could change requirements.txt to install python-doctr[tf] instead of installing directly from GitHub. WDYT?

@fg-mindee fg-mindee self-assigned this Sep 28, 2021
@fg-mindee fg-mindee added the ext: demo Related to demo folder label Sep 28, 2021
@fg-mindee fg-mindee added this to the 0.4.0 milestone Sep 28, 2021
Copy link
Contributor

@fg-mindee fg-mindee 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 demo & the bug fix @osanseviero 🙏

To answer your questions:

  • we have an upcoming release in a few days so the page synthesis will be on a stable release!
  • you're correct about the package ref in the demo, but for the same reason than your first point, I think it might be best to stay aligned with the main branch on Github, don't you think?

I added a comment to use the extras to have a fully compatible TF environment, let me know what you think!

demo/requirements.txt Outdated Show resolved Hide resolved
@fg-mindee fg-mindee added the topic: build Related to dependencies and build label Sep 28, 2021
@codecov
Copy link

codecov bot commented Sep 28, 2021

Codecov Report

Merging #492 (7cfa99a) into main (9126a4e) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main     #492   +/-   ##
=======================================
  Coverage   94.99%   94.99%           
=======================================
  Files         108      108           
  Lines        4177     4177           
=======================================
  Hits         3968     3968           
  Misses        209      209           
Flag Coverage Δ
unittests 94.99% <ø> (ø)

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


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9126a4e...7cfa99a. Read the comment docs.

@osanseviero
Copy link
Contributor Author

Thanks for your input! I changed the requirements as you suggested and removed the TF requirements since they are part of the extras.

I'm glad you liked the demo!

Copy link
Contributor

@fg-mindee fg-mindee left a comment

Choose a reason for hiding this comment

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

Much appreciated Omar!

@fg-mindee
Copy link
Contributor

@osanseviero by any chance, is there a markdown badge or something similar for huggingface spaces demo?
That could be a cool thing to include in the README in another PR 😁

@fg-mindee fg-mindee merged commit e01e8fd into mindee:main Sep 28, 2021
@fg-mindee
Copy link
Contributor

For reference: cf #426

@osanseviero osanseviero deleted the patch-1 branch September 29, 2021 10:31
osanseviero added a commit to osanseviero/doctr that referenced this pull request Sep 29, 2021
fg-mindee pushed a commit that referenced this pull request Sep 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ext: demo Related to demo folder topic: build Related to dependencies and build
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants