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

Date comparison bug #12

Closed
cegme opened this issue Jan 31, 2018 · 2 comments
Closed

Date comparison bug #12

cegme opened this issue Jan 31, 2018 · 2 comments

Comments

@cegme
Copy link

cegme commented Jan 31, 2018

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
Copy link
Collaborator

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
Copy link
Member

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

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

No branches or pull requests

3 participants