openeventdata / UniversalPetrarch

Language-agnostic political event coding using universal dependencies
MIT License
18 stars 9 forks source link

Date comparison bug #12

Closed cegme closed 6 years ago

cegme commented 6 years ago

There is a bug in the date comparison present in the check_date function.

This bug is described in openeventdata/petrarch2#48. Python is comparing a string and an integer. When there is a comparison between these two types the "names" of the types are compared (e.g. int < str).

The instances where this bug occurs are here, here, here, and here.

Strangely, A simple fix in Petrarch 2 (turning the string cur_date to an integer for comparison) breaks the existing test cases.

JingL1014 commented 6 years ago

The test cases should be fixed. When constructing the sentence, the date should be converted to integer first as in petrarch2.py. The expected behavior is comparing the dstr_to_ordate(curdate) to the Integer date.

>>> date1 = '081315'
>>> PETRreader.dstr_to_ordate(date1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "PETRreader.py", line 1844, in dstr_to_ordate
    raise DateError
PETRreader.DateError
>>> PETRreader.dstr_to_ordate('081215') >=  PETRreader.dstr_to_ordate('150518')
False

In the above examples, '081315' is an invalid date string, so the code should throw error. And date '150518' should be larger than date '081215'

ahalterman commented 6 years ago

It sounds like this is resolved. Closing, but re-open if the problem is still ongoing.