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

many more Ocropy improvements #17

Merged
merged 19 commits into from
Oct 29, 2019
Merged

many more Ocropy improvements #17

merged 19 commits into from
Oct 29, 2019

Conversation

bertsky
Copy link
Collaborator

@bertsky bertsky commented Oct 22, 2019

Note: this depends on OCR-D/core#311 and OCR-D/core#327 (which is already in core:edge but not merged to master and not released yet – except via ocrd/core:edge on Dockerhub). So merging probably does not make sense yet.

bertsky and others added 17 commits September 24, 2019 15:29
- common.remove_noise: in addition to ocrolib.common.remove_noise
  on the binary image (which removes small foreground components),
  run on the inverted image (removing small "background components",
  i.e. white specks in letters)
- denoise: increase default maxsize, and re-parameterize to unit pt
  (points), making it independent of image resolution
- when cropping a clipped segment image from its parent image,
  do not use the segment's width and height in absolute coordinates
  (with merely the offset in relative coordinates), but instead
  determine the segment's bounding box only in relative coordinates
- after cropping, if the segment has an orientation angle,
  then rotate the segment image accordingly (but subtract any
  higher level angle already present)

(Both fixes are required to meet the expectations of the core image
 API; otherwise relative coordinates cannot be calculated correctly
 for the generated images.)
- when an AlternativeImage already exists for a TextLine
  and hence its coordinates must be compared to the region
  segmentation on the line level instead of the region level,
  by cropping region labels to the same bounding box,
  do not use the line's width and height in absolute coordinates
  (with merely the offset in relative coordinates), but instead
  determine the line's bounding box only in relative coordinates
- when creating a mask array from a polygon, ensure to also fill
  the outline (not just the interior)
- when thresholding the size of a contour's parts, re-use and
  respect the existing threshold parameter
- filter deskewed images only on the level of operation
- respect angle already applied on parent level by
  * rotating from that actual to the annotated target angle
  * annotating the sum of both in the result
- do not propagate image features deskewed and rotated-X
@bertsky
Copy link
Collaborator Author

bertsky commented Oct 26, 2019

@finkf I updated the requirement to ocrd>=2.0.0a1, which is the PyPI release line for core:edge as detailed above. So this can be merged now.

Do you want me to release it on PyPI as well? (I don't see any versions there yet, also for the old package name. PyPI deployment is a requirement for many kinds of use cases / users...)

@kba
Copy link
Contributor

kba commented Oct 26, 2019

Do you want me to release it on PyPI as well? (I don't see any versions there yet, also for the old package name. PyPI deployment is a requirement for many kinds of use cases / users...)

I will do that now, as v0.0.5. Happy to turn the package over to you and support you if you're unfamiliar with pypi.

@kba
Copy link
Contributor

kba commented Oct 26, 2019

@bertsky
Copy link
Collaborator Author

bertsky commented Oct 26, 2019

@finkf same offer from me. Also: I can help you rework the rest of the repo to get ready for core 1.0 / 2.0, and end up with a good master – if you like.

Regarding core, I have seen a significant change for post-correction after ocrd==1.0.0b10: the TextEquiv.conf attribute is no longer a float, but a string again. (Remember, we used to have a problem in core at that point with newer generateDS code not correctly parsing string conf from PAGE strings into the PAGE DOM. The fix included a change on the other side, though: Now it generates the DOM with conf as string as well. So you have to convert to float yourself to get the old behaviour on the receiving end during post-correction. For examples of how I fixed this in my repos, see ocrd_keraslm and cor-asv-ann. The reason behind this is generateDS does not support conversion from simpleType for attributes.)

And in case you do not already know: ocrd 1.0 was a bit premature, because the much needed image API fixes (which have already been around for a while) already break it. So we introduced a new branch edge on core and on ocrd_tesserocr until we know these can be trusted themselves. To remedy installation of these, they got pre-released under ocrd==2.0.0a1 and ocrd_tesserocr==0.5.0 now. I know – lots of complexity, very confusing. Sorry!

@finkf
Copy link
Contributor

finkf commented Oct 28, 2019

Yes, help would be appreciated. I think, that I will merge this and add my changes to devel as well.
Then we can check things and update master.

As to the TextEquiv.conf I think that non of our post-correction python code touches the confidence value, so we should be good.

@bertsky
Copy link
Collaborator Author

bertsky commented Oct 28, 2019

Yes, help would be appreciated. I think, that I will merge this and add my changes to devel as well.
Then we can check things and update master.

Ok, let me know when I should take a look.

As to the TextEquiv.conf I think that non of our post-correction python code touches the confidence value, so we should be good.

Are you quite sure? It seems you are using word confidences the same way I was using glyph confidences. That line is going to fail after 1.0.0b10. You will need something like

conf = min([float(te0(x).conf or "1.") for x in word.region])

@finkf
Copy link
Contributor

finkf commented Oct 28, 2019

Yeah. You are right. I will fix this (after the merge). So I will merge this now, and fix TextEquiv.conf afterwards. OK?

@bertsky
Copy link
Collaborator Author

bertsky commented Oct 28, 2019

Whatever suits you better. (The PR adds the 2.0.0a1 requirement, so it would be "cleaner" to add all other updates here first. But if you already have changes yourself, it's probably not worth the effort, and synchronize afterwards.)

@finkf
Copy link
Contributor

finkf commented Oct 28, 2019

Yes. I guess you are right

@lgtm-com
Copy link

lgtm-com bot commented Oct 29, 2019

This pull request introduces 2 alerts and fixes 1 when merging 590c2f2 into a0dd028 - view on LGTM.com

new alerts:

  • 1 for Unused import
  • 1 for Wrong number of arguments in a call

fixed alerts:

  • 1 for Syntax error

@finkf finkf merged commit 590c2f2 into cisocrgroup:dev Oct 29, 2019
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 this pull request may close these issues.

3 participants