-
Notifications
You must be signed in to change notification settings - Fork 1
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
Port to ocrd core version 3.0.0 #5
base: fix-alpha-shape
Are you sure you want to change the base?
Conversation
Port to v3
if level == 'page': | ||
try: | ||
ret.append(self.process_page(page, page_image, page_xywh, zoom, page_id, output_file_id)) | ||
except ValueError as e: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bertsky Do we even want to catch these or should we let them explode and let core do the error handling? For page-wide binarization, I think we probably should because that means the page failed. But for region and line, we might have rogue instances of zero size but all the other regions/lines might be fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will soon catch all exceptions on the page level in core. So this should not be handled here.
Regarding lower-level error handling: We have discussed this before, but partial failures across a page in general mean we also must be able to cope with partial annotation (incremental processors). We have no real solution ATM.
But since this PR just preserves the current behaviour (skipping partial failures regardless of level), and there are also other possible causes to catch in core, let's keep it like this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ocrd-cis-ocropy-binarize
with new API LGTM!
Co-authored-by: Konstantin Baierer <[email protected]>
Co-authored-by: Konstantin Baierer <[email protected]>
Co-authored-by: Konstantin Baierer <[email protected]>
Co-authored-by: Konstantin Baierer <[email protected]>
binarize: don't conflate region/lines seg, pass output_file_id
I will push a simple Github Actions workflow to this PR to execute the tests. |
Co-authored-by: Robert Sachunsky <[email protected]>
Co-authored-by: Robert Sachunsky <[email protected]>
@bertsky, I think the GitHub Actions workflow is not triggered until you have that in your fork's |
No, AFAIK GH Actions must be activated for each GH repo/fork individually. So in this case, it would be your fork. Then once I merged here, I have to enable on my fork. And once fix-alpha-shape gets merged upstream, GHA would need to be activated there. |
Wow. So with that we now know that we will get more problems on Ocrolib starting with Python 3.9:
But at least 3.8 runs through (and fast!) |
It was probably me who messed up a bit and created the |
Ok, so what do we do next? Debugging CircleCI seems tiresome, perhaps we should just deactivate that (keeping the config file). But then we should also add a CD to GHA. (The credentials for |
I will see if I can find a fast fix for that. But I will have to modify the ocrolib slightly to make NaN work with higher Python versions. EDIT: All tests pass now after 224e86f and a397531. Not sure if a397531 was needed since tests pass regardless. |
Yes. |
Already migrated processors: