CopticScriptorium / coptic-nlp

Coptic NLP pipeline page and utilities
Apache License 2.0
14 stars 5 forks source link

nlp producing lang tags on morphs in a separate column #13

Closed ctschroeder closed 5 years ago

ctschroeder commented 5 years ago

On the integration of the NLP API with gitdox: In some of the Johannes documents, I see a separate column labeled morph_xml:lang for language tags on the morph level. All lang tags are supposed to be in the same column. I am manually moving them from one column to the other.

amir-zeldes commented 5 years ago

I see. This must have changed when we added the possibility of multiple columns with the same name, so now they don't overwrite each other.

I just fixed this with a hardwired patch, but I'm not happy with this solution, since in principle, if there are annotations:

<morph xml:lang...

And

<norm xml:lang...

It's correct to put them in different columns (from a generic data model perspective). It's a coincidence of Scriptorium data that xml:lang can't overlap for morph and norm, but in principle, two conflicting values could apply at the same time, so not everything should be collapsed this way.

I'm leaving this issue open to remind us to deal with this at some point, this behavior should probably be configurable for a subset of annotation names. @ctschroeder let me know if you still see the problem, it should be fixed for now.

amir-zeldes commented 5 years ago

Moving to gucorpling/gitdox#133

ctschroeder commented 5 years ago

Ok thanks. I understand what you’re saying about data models but in ANNIS it would be a real pain to have to search for 2 layers of annotation. Thanks so much for the patch.

amir-zeldes commented 5 years ago

Oh, there's no question there should be only one lang annotation in ANNIS etc., it's just an incompatibility between the NLP output's data model and what we want in ANNIS. It's not too hard to change one into the other, it's just that right now it's hard wired to the XML attribute name xml:lang inside the GitDox code, and ideally it should be configurable somewhere. Nothing urgent though, just something to keep an eye on and fix at some point.