huji-nlp / ucca

Universal Conceptual Cognitive Annotation (UCCA)
https://universalconceptualcognitiveannotation.github.io/
GNU General Public License v3.0
20 stars 20 forks source link

Ucca1.3.5 #104

Closed lovodkin93 closed 3 years ago

lovodkin93 commented 3 years ago

updated the test_validation.py and the vallidation.py files. Please don't merge the test_evaluation.py file.

lovodkin93 commented 3 years ago

updated the code. Pay attention that there are still going to be two errors - but those errors are correct as the first one (discontiguous-True) generates a UCCA graph where a D unit has a G child and also C and E units are siblings with G unit (which is also illegal), which is illegal, and the second error (l1_passage-True) generates a UCCA graph where a H unit has H and L children, which is also illegal

lovodkin93 commented 3 years ago

Updated the discontiguous and the l1_passage tests in test_validation.py (and all the relevant assertions)

danielhers commented 3 years ago

Great! There seem to be conflicts now though... do you understand why?

lovodkin93 commented 3 years ago

Great! There seem to be conflicts now though... do you understand why?

it seems to be caused by the 'main_rel' parameter that is being checked in the ucca/tests/test_constructions.py file. In the version I cloned of UCCA, this parameter wasn't included. For now I will add it to the changes I've made. Please let me know if there are any problems.

lovodkin93 commented 3 years ago

resolved the conflicts.

danielhers commented 3 years ago

Thanks again, but now the tests fail again... because you changed the path to the XML file to include ../../. I think the test code assumes you run it from the project root as pytest . or pytest ucca/tests and not from inside the ucca/tests directory.

lovodkin93 commented 3 years ago

Hey, you are right. On my computer it works only with "../../", and I forgot to get rid of it before committing. Now it should be okay. Unless the "main_rel" causes problems.