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

Migrate static data from github to monitoring middleware. #1033

Merged
merged 3 commits into from
Aug 31, 2022

Conversation

marvinmindee
Copy link
Contributor

Static data like models and images are now downloaded from a middleware and not from github releases, to enable more flexibility and monitor model downloads.

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 @marvinmindee :)

Quick question though:

  • I understand clearly the advantage of monitoring the downloads of models (perhaps there is a feature on Github to monitor asset download but I'm not sure)
  • but for the rest (images, etc.), what would be the advantage?

Thanks 🙏

@marvinmindee
Copy link
Contributor Author

Hello, we also added he images so that all static data are accessed the same way and we don't have things dowloaded from multiple places (especially when we'll want to add other static files, it's better if we don't have to think about whether it should be on github/release or doctr-static). It also leaves flexibility for later!

@felixdittrich92
Copy link
Contributor

felixdittrich92 commented Aug 30, 2022

@frgfm @marvinmindee If we switch the models to doctr-static i agree that we should do this for all static data so we have all in one place 👍

one failing test left to fix :)

@codecov
Copy link

codecov bot commented Aug 31, 2022

Codecov Report

Merging #1033 (5fe87d4) into main (61d0f1c) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #1033      +/-   ##
==========================================
- Coverage   94.93%   94.91%   -0.02%     
==========================================
  Files         135      135              
  Lines        5590     5590              
==========================================
- Hits         5307     5306       -1     
- Misses        283      284       +1     
Flag Coverage Δ
unittests 94.91% <100.00%> (-0.02%) ⬇️

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

Impacted Files Coverage Δ
doctr/datasets/cord.py 97.72% <ø> (ø)
doctr/datasets/funsd.py 97.43% <ø> (ø)
doctr/datasets/ic03.py 97.43% <ø> (ø)
doctr/datasets/ic13.py 96.77% <ø> (ø)
doctr/datasets/iiit5k.py 96.96% <ø> (ø)
doctr/datasets/imgur5k.py 93.33% <ø> (ø)
doctr/datasets/sroie.py 97.36% <ø> (ø)
doctr/datasets/svhn.py 95.34% <ø> (ø)
doctr/datasets/svt.py 97.29% <ø> (ø)
doctr/datasets/synthtext.py 94.73% <ø> (ø)
... and 18 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

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.

Thanks, LGTM

@charlesmindee charlesmindee merged commit a3513b0 into main Aug 31, 2022
@charlesmindee charlesmindee deleted the doctr-static branch August 31, 2022 14:04
@github-actions
Copy link

Hey @charlesmindee 👋
You merged this PR, but it is not correctly labeled. The list of valid labels is available at https://github.com/mindee/doctr/blob/main/.github/verify_pr_labels.py

@felixdittrich92 felixdittrich92 added this to the 0.6.0 milestone Aug 31, 2022
@felixdittrich92 felixdittrich92 added type: enhancement Improvement topic: build Related to dependencies and build ext: tests Related to tests folder labels Aug 31, 2022
@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: tests Related to tests folder topic: build Related to dependencies and build type: enhancement Improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants