qurator-spk / sbb_textline_detection

Detect textlines in document images
Apache License 2.0
90 stars 18 forks source link

Rename ocrd_sbb_textline_detector #9

Closed stweil closed 4 years ago

stweil commented 4 years ago

I suggest to rename ocrd_sbb_textline_detector to ocrd-sbb-textline-detector or ocrd-sbb-segment-line which fit better to the OCR-D conventions.

CC'ing @kba.

cneud commented 4 years ago

WONTFIX, because this tools is from another funded project, not OCR-D, and we want/need to be transparent ;-)

stweil commented 4 years ago

I am sorry, but I don't understand why replacing _ by - would be in-transparent?

kba commented 4 years ago

I think @cneud meant that we won't rebrand non-OCR-D tools as ocrd-?

cneud commented 4 years ago

I think @cneud meant that we won't rebrand non-OCR-D tools as ocrd-?

Precisely! This is NOT an OCR-D effort, and we are required for funders to be clear about this! We cannot simply rebrand products of other funded projects.

cneud commented 4 years ago

It is a voluntary concession by SBB to also issue QURATOR tools with OCR-D compliant interfaces (where possible).

stweil commented 4 years ago

For me, the ocrd- prefix just indicates that it is a tool which fits into the OCR-D toolchain. I did not interpret it as an indicator that it was funded in the OCR-D context.

Why not omit ocrd_ as well, and use - in the name of the script nevertheless?

cneud commented 4 years ago

Maybe, but we have another project here with another naming scheme in use. Does it really matter?

kba commented 4 years ago

@cneud Just to clarify, @stweil is referring to the executable ocrd_sbb_textline_detector in the ocrd-tool.json. This is already prefixed ocrd_ and should indeed follow the same convention as all the other tools, i.e. ocrd and kebab-case.

cneud commented 4 years ago

Yeah I think we can rename the OCR-D-Processor.

But just to be aware that there is a full framework context with specs (incl. naming conventions etc.) behind here, as well - just not the one of OCR-D ;-)

stweil commented 4 years ago

I only talked about your already existing OCR-D script and am a little bit surprised about the reaction.

cneud commented 4 years ago

We already try in QURATOR what we can to be as compatible to OCR-D, see e.g. ocrd_calamari, dinglehopper. But this is all extra, and we simply have other obligations/priorities in the other project, too.

I will make another PR about the OCR-D script.

kba commented 4 years ago

I will make another PR about the OCR-D script.

10

cneud commented 4 years ago

I will make another PR about the OCR-D script.

10

OK you beat me to it ;-)

bertsky commented 4 years ago

It appears the above fix is only half done:

  File "qurator/sbb_textline_detector/ocrd_cli.py", line 34, in __init__
    kwargs['ocrd_tool'] = OCRD_TOOL['tools'][TOOL]
KeyError: 'ocrd_sbb_textline_detector'

(So it's not enough to change the name in the tool json and setup.py entry points, there is another mention in ocrd_cli.py waiting to be renamed as well.)

Please re-open!

kba commented 4 years ago

Remaining mention in ocrd_cli.py is in #12.

mikegerber commented 4 years ago

Tested and merged #12

mikegerber commented 4 years ago

Some opinions as the author of the OCR-D CLI here (My colleague @vahidrezanezhad is responsible for most of the rest):

stweil commented 4 years ago

@mikegerber, I am sorry that I approved a pull request without testing it. To help avoiding such problems in the future, I suggest to add some continuous integration tests. A first one is added by pull request #13.

See https://travis-ci.org/stweil/sbb_textline_detector/builds for some results.