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

New installations are broken because of upstream issue for detectron2 #26

Closed
stweil opened this issue Jul 3, 2023 · 7 comments · Fixed by #28
Closed

New installations are broken because of upstream issue for detectron2 #26

stweil opened this issue Jul 3, 2023 · 7 comments · Fixed by #28

Comments

@stweil
Copy link
Contributor

stweil commented Jul 3, 2023

Building works, but running ocrd-detectron2-segment with make check raises an exception.

See related upstream issue facebookresearch/detectron2#5010.

Until upstream is fixed, pip install -U "pillow<10.0.0" helps.

@bertsky
Copy link
Owner

bertsky commented Jul 3, 2023

Strange that this did not hit us in the Docker build (also Py38).

Since it's unlikely this will get fixed in detectron2 any time soon (last release is from 2021), but we do not use this anywhere else in ocrd_all, would you recommend adding pillow<10.0 here in requirements.txt?

@stweil
Copy link
Contributor Author

stweil commented Jul 3, 2023

Pillow 10.0.0 (which causes the breakage) was released on July 1, so it's not surprising that your or my previous builds passed fine. And yes, updating the requirements seems to be the only possible solution. Instead of updating the requirements (which would cause downgrades of pillow during the build process), it might be better to patch detectron2 after it was installed.

@bertsky
Copy link
Owner

bertsky commented Jul 3, 2023

was released on July 1, so it's not surprising that your or my previous builds passed fine.

It is surprising – the latest Docker build is just 2h ago.

Instead of updating the requirements (which would cause downgrades of pillow during the build process), it might be better to patch detectron2 after it was installed.

I would not know how to do this (properly). Using make deps, Detectron2 gets pulled from git and then compiled as a temporary wheel before getting installed into the current venv...

stweil added a commit to stweil/ocrd_detectron2 that referenced this issue Jul 3, 2023
@bertsky
Copy link
Owner

bertsky commented Jul 3, 2023

I think I found a clean way.

Can you please try #28?

@stweil
Copy link
Contributor Author

stweil commented Jul 3, 2023

The pull requests #27 and #28 basically do the same thing to fix this issue. They only use slightly different ways to find the location of the installed detectron2 and to patch the code. It's your choice which one you prefer.

@rayryeng
Copy link

detectron2 was recently updated (July 6, 2023) so that this issue is now fixed. You should be able to use Pillow==10.0.0 now.

facebookresearch/detectron2@ff53992

@bertsky
Copy link
Owner

bertsky commented Jul 10, 2023

Thanks @rayryeng for letting me know!

Unfortunately, we still require released version v0.6 here, and there has been no release since then for over 2 years. I cannot see what ramifications the switch to master would have for (the performance of) all the models that we redistribute here...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants