Skip to content

Conversation

@bertsky
Copy link
Collaborator

@bertsky bertsky commented May 27, 2020

…to ensure that the given polygon is fully contained
within that parent's polygon.

Fixes #489.

But please don't merge yet, I have another, better idea

…to ensure that the given polygon is fully contained
within that parent's polygon.
@bertsky bertsky requested a review from kba May 27, 2020 14:44
@codecov-commenter
Copy link

codecov-commenter commented May 27, 2020

Codecov Report

Merging #492 into master will decrease coverage by 1.09%.
The diff coverage is 18.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #492      +/-   ##
==========================================
- Coverage   80.12%   79.03%   -1.10%     
==========================================
  Files          40       40              
  Lines        2390     2485      +95     
  Branches      443      482      +39     
==========================================
+ Hits         1915     1964      +49     
- Misses        403      433      +30     
- Partials       72       88      +16     
Impacted Files Coverage Δ
ocrd_utils/ocrd_utils/__init__.py 62.31% <18.18%> (-3.70%) ⬇️
ocrd_validators/ocrd_validators/page_validator.py 77.19% <0.00%> (-4.06%) ⬇️
ocrd/ocrd/cli/workspace.py 68.86% <0.00%> (-2.94%) ⬇️
ocrd_models/ocrd_models/ocrd_mets.py 93.92% <0.00%> (-0.76%) ⬇️
ocrd/ocrd/workspace.py 58.02% <0.00%> (ø)

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 e008991...d952f6c. Read the comment docs.

interp = childp.intersection(parentp)
if interp.is_empty:
# FIXME: we need a better strategy against this
raise Exception("intersection of would-be segment with parent is empty")
Copy link
Member

Choose a reason for hiding this comment

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

Do we want to be robust against such blatant errors?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yes, probably just show a warning. But we have no context for localizing the message at that point...

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.

ocrd_utils.coordinates_for_segment: clip to parent?

3 participants