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

coordinates_for_segment: add optional parent_element… #492

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

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.

ocrd_utils/ocrd_utils/__init__.py Show resolved Hide resolved
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...

ocrd_utils/requirements.txt Show resolved Hide resolved
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