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_cli_wrap_processor: drop passing ocrd_tool=None #998

Closed
bertsky opened this issue Mar 15, 2023 · 1 comment · Fixed by #999
Closed

ocrd_cli_wrap_processor: drop passing ocrd_tool=None #998

bertsky opened this issue Mar 15, 2023 · 1 comment · Fixed by #999

Comments

@bertsky
Copy link
Collaborator

bertsky commented Mar 15, 2023

We seem to have a bug from very early on that never surfaced:

Obviously, nowhere in the CLI params there is a setting for the ocrd_tool.

However, our Processor decorator ocrd_cli_wrap_processor does take such a kwarg:

ocrd_tool=None,

It does nothing but pass it on to run_processor:

run_processor(processorClass, ocrd_tool, mets, workspace=workspace, **kwargs)

In that context, ocrd_tool could be meaningful (e.g. when running a processor from API):

ocrd_tool=None,

So it gets passed onto the processor constructor:

processor = get_processor(
processor_class=processorClass,
parameter=parameter,
workspace=workspace,
ocrd_tool=ocrd_tool,

Still, this is not usually a problem, because all of our Processor subclasses simply override that kwarg.

But what if they just default to their own value?

IMO the top-level ocrd_tool param is wrong must be removed.

@kba
Copy link
Member

kba commented Mar 15, 2023

Thanks for digging into and solving this. I am not sure why we had the ocrd_tool in there in the first place.

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 a pull request may close this issue.

2 participants