utils/image: add polygon_for_parent w/ a test

[kba]

trafficstars

[codecov-commenter]

Codecov Report

Merging #577 into master will decrease coverage by 0.15%. The diff coverage is 37.50%.

@@            Coverage Diff             @@
##           master     #577      +/-   ##
==========================================
- Coverage   84.14%   83.98%   -0.16%     
==========================================
  Files          49       49              
  Lines        2776     2792      +16     
  Branches      541      546       +5     
==========================================
+ Hits         2336     2345       +9     
- Misses        342      344       +2     
- Partials       98      103       +5     

Impacted Files	Coverage Δ
ocrd_utils/ocrd_utils/__init__.py	100.00% <ø> (ø)
ocrd_utils/ocrd_utils/image.py	54.97% <37.50%> (-1.60%)	:arrow_down:
ocrd/ocrd/resolver.py	96.55% <0.00%> (+3.44%)	:arrow_up:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update e41ba75...d03e2cd. Read the comment docs.

[kba]

We have been discussing GT validation of coordinates again today, as well as possible applications for ocrd-sanitize here, that's why I wanted to understand how to use that function. I see this as a step towards the eventual goal of PAGE-level validation/cl;ipping, the adaptions to enable better error reporting in GdsCollector is in the same vein. I just momentarily wasn't aware of #492. Can't we not try to bring the coordinate logic from tesserocr into OCR-D/core first, while working towards PAGE-level methods?

[bertsky]

Can't we ~~not~~ try to bring the coordinate logic from tesserocr into OCR-D/core first, while working towards PAGE-level methods?

But the exception was merely an experiment. We don't want that in production code. So not only would we have to add the call to nearly every processor, it would also have to catch the exception each time. That's just too much effort IMO.

https://github.com/OCR-D/ocrd_tesserocr/pull/145 already demonstrates that ocrd_tesserocr correctly would have to catch the exception, ignoring the candidate. (I just rewrote with return type None.)

Thinking about this from the PAGE model is a different picture, though. Functions like add_TextRegion would have to throw some CoordsNotInParentException which needs to be handled differently in the caller/processor.

I think you might draw 2 different conclusions from that:

1. Moving the check into the PAGE model implicitly is not worth it, because the processors still need to be updated (catching the exception), so one might as well keep it a separate function and opt-in (tolerating processors which don't use it and thus may produce inconsistent coords).
2. Moving the check into the PAGE model implicitly allows us to enter a (non-production) phase where users will see exceptions as long as the respective processors have not been fixed.

[bertsky]

Thinking about this from the PAGE model is a different picture, though. Functions like add_TextRegion would have to throw some CoordsNotInParentException which needs to be handled differently in the caller/processor.

I think you might draw 2 different conclusions from that:

1. Moving the check into the PAGE model implicitly is not worth it, because the processors still need to be updated (catching the exception), so one might as well keep it a separate function and opt-in (tolerating processors which don't use it and thus may produce inconsistent coords).
2. Moving the check into the PAGE model implicitly allows us to enter a (non-production) phase where users will see exceptions as long as the respective processors have not been fixed.

After today's tech call, the picture has become even larger. #576 offers a new alley for making implementors aware of these problems – by feeding PAGE validation failures (along with call stack context if necessary) into dedicated loggers. But most of these failures very likely would result in subsequent crashes of the follow-up processors anyway (because we cannot expect to make all of them robust against all sorts of input invalidities, even in the long term). Plus we already face the problem that processors can fail (due to exception or exit) on single pages (often only after computing a long series of pages correctly). This is not tolerable for a system aiming at productive use. So is option 2 above. Therefore, we already need a mechanism of catching a processor's exceptions and falling back to (say) the input page as output page. That very mechanism could then be used to let exceptions fall through when requested. This could be controlled on the command line with a new option (say --debug). The single place to handle this would be ocrd.Processor.process. (But for the page-wise fallback we would also need #322.) Processor implementors could then run (and make test) their code with --debug and start catching the exceptions from the PAGE model themselves accordingly.