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

Relax Pillow and OpenCV version bounds. #1373

Merged
merged 1 commit into from
Nov 6, 2023
Merged

Conversation

nh2
Copy link
Contributor

@nh2 nh2 commented Nov 5, 2023

For Pillow, the change in commit

e04e183 - chore: apply PIL major changes and increase min version specifier (#1237)

that does the font.getsize() -> font.getbbox() migration is definitely already available in Pillow 9.2.0, see

https://pillow.readthedocs.io/en/stable/deprecations.html#font-size-and-offset-methods

Deprecated since version 9.2.0.

@felixdittrich92
Copy link
Contributor

Hi @nh2 👋,
Thanks for the PR.
What's the reason to increase the lower bound of opencv ? :)

@felixdittrich92 felixdittrich92 self-assigned this Nov 5, 2023
@felixdittrich92 felixdittrich92 added this to the 0.8.0 milestone Nov 5, 2023
@felixdittrich92 felixdittrich92 added topic: build Related to dependencies and build type: misc Miscellaneous labels Nov 5, 2023
Copy link

codecov bot commented Nov 5, 2023

Codecov Report

Merging #1373 (f1e34ad) into main (4cadd8d) will not change coverage.
Report is 1 commits behind head on main.
The diff coverage is n/a.

❗ Current head f1e34ad differs from pull request most recent head d4e21cf. Consider uploading reports for the commit d4e21cf to get more accurate results

@@           Coverage Diff           @@
##             main    #1373   +/-   ##
=======================================
  Coverage   95.81%   95.81%           
=======================================
  Files         155      155           
  Lines        6952     6952           
=======================================
  Hits         6661     6661           
  Misses        291      291           
Flag Coverage Δ
unittests 95.81% <ø> (ø)

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

@felixdittrich92
Copy link
Contributor

Please update your branch from main :)

For Pillow, the change in commit

    e04e183 - chore: apply PIL major changes and increase min version specifier (mindee#1237)

that does the `font.getsize()` -> `font.getbbox()` migration
is definitely already available in Pillow 9.2.0, see

https://pillow.readthedocs.io/en/stable/deprecations.html#font-size-and-offset-methods

> Deprecated since version 9.2.0.
@nh2
Copy link
Contributor Author

nh2 commented Nov 6, 2023

What's the reason to increase the lower bound of opencv ? :)

@felixdittrich92 Ah, sorry, that is a leftover. I had played with the OpenCV version bounds as well because on NixOS it couldn't find OpenCV. But it turned out that's because there it's called just opencv, not opencv-python. I took it out now

Please update your branch from main :)

Done.


FYI, I have packaged doctr for nixpkgs: NixOS/nixpkgs#265735

This might make it easy for more users to get a fully working doctr with all recursive system dependencies easily with Nix.

Copy link
Contributor

@felixdittrich92 felixdittrich92 left a comment

Choose a reason for hiding this comment

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

Thanks sounds great @nh2 👍
If your mentioned PR is merged feel free to open a follow up PR we should add it to the installation docs as option :)

@felixdittrich92 felixdittrich92 merged commit e00cc79 into mindee:main Nov 6, 2023
67 of 68 checks passed
@nh2
Copy link
Contributor Author

nh2 commented Nov 6, 2023

If your mentioned PR is merged feel free to open a follow up PR we should add it to the installation docs as option :)

@felixdittrich92 Ah yes, I just spotted from #1322 that you're interested in packaging options. Nix can be a convenient alterntaive to docker.

Here's the one-liner invocation that uses the WIP commit from my PR; it drops you into a Python shell where doctr is working, with all dependencies reproducibly pinned (assuming Nix is installed):

NIX_PATH=nixpkgs=https://github.com/NixOS/nixpkgs/archive/6c069433b46584559bdadcb4d74fb760308aabb0.tar.gz nix-shell -p 'python3.withPackages (ps: with ps; [ doctr ])' --run python3
>>> import doctr
>>> doctr.__version__
'0.7.0a0'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: build Related to dependencies and build type: misc Miscellaneous
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants