-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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 the build system to poetry #1023
base: master
Are you sure you want to change the base?
Conversation
I would be very happy about this changes :-) |
Great! I will try to get it back to speed today. Haven't looked at it in a bit :) |
I've adjusted and cleaned up a bit. The doc is also updated to reflect the buildmatrix. I did not add test for all combinations. |
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.
Great work
Thanks for that fast response ! |
Lets hope that it will be approved and merged soon 💯 |
@JaidedTeam is there any possibility to do a review and a release during the next time ? |
Yes please, this would be very useful. We're using EasyOCR in an environment where no GPU support is available so we're forced to take down a huge slice of useless dependencies space. |
@SamuelYvon Seems like this branch has some conflicts |
It does but nobody from JaidedAI seems concerned about it, so I'll put this in my backlog. If a maintainer that can approve / merge sees this, feel free to ping me and I'll fix things. |
@JaidedTeam This PR is open since may and i still need it :-) Any possibility to review and merge it soon ? |
Still no feedback from the @JaidedTeam 👎 |
Hey @rkcosmos it would be really great if you get this thing done ! |
What is this PR
I suggest migrating the build system from the traditional setup to using a more modern / advanced tool such as poetry. This work is not complete yet and I have a few things to fix.
Motivation
The motivation for this is that it is very hard to integrate easyocr in external projects that require
opencv
or / and pytorch. This is the case of many machine learning solutions that use different versions (specifically, the distribution of opencv, or pytorch in CPU only mode). These changes keep the same dependencies for the default pip package (I'm unsure where / how you built it in the past) but allow users of easyocr as a library to tweak the exact dependencies.Unintended Consequences
Poetry has some limitations. In this case, we would like to force the choice of any of the opencv choices as well as torch, which is not a supported use case for poetry. This has to be managed by the user. As a preventative measure, I added a check in the main
__init__.py
file that both libraries can be importedWhat's left
Bug addressed
@JaidedTeam Before I finish working on this, I want to make sure this is a change you want to see in the library. If you have no interest, I will maintain my own fork for my own purposes, but I truly believe this is a better solution than the current requirements.txt