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

ocrd validate: accept --mets-basename, bashlib: use that #925

Closed
wants to merge 2 commits into from

Conversation

kba
Copy link
Member

@kba kba commented Oct 21, 2022

When calling a bashlib processor like ocrd-olena-binarize with a workspace backed by a METS not called mets.xml, this leads to an error because we call ocrd validate tasks at the end of the bashlb argument parsing and it only specifies the --workspace (directory) so fails to instantiate the workspace, or even uses the wrong file.

I would like to consolidate the options to a single --mets option like we did for the processors and ocrd workspace option, but that will probably best happen in conjunction with the METS server change.

@bertsky bertsky self-requested a review October 21, 2022 11:32
Copy link
Collaborator

@bertsky bertsky left a comment

Choose a reason for hiding this comment

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

Good catch!

We should probably also add some basename tests with the dummy processor (like test_mets_basename and test_ocrd_dummy.TestDummyProcessor combined).

Also, we have next to no tests for bashlib processing yet.

BTW, before ocrd-olena-binarize (and the rest of them) can actually handle the non-default basename case correctly, we would still have to rewrite all the ocrd workspace calls to something like ocrd workspace -m $(basename ${ocrd__argv[mets_file]}).

And so to facilitate that, in turn, we should probably add something like ocrd__workspace function to lib.bash (which would work both before and after the usual cd call)...

ocrd/ocrd/cli/validate.py Outdated Show resolved Hide resolved
Co-authored-by: Robert Sachunsky <38561704+bertsky@users.noreply.github.com>
@kba
Copy link
Member Author

kba commented Oct 21, 2022

Merged with the basename typo fix, but amended, so GH thinks there's unmerged changes here.

@kba kba closed this Oct 21, 2022
@kba kba deleted the bashlib-validate-mets-basename branch October 21, 2022 16:33
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.

2 participants