Skip to content

Clamp the day of month against 31, not 12, in extract_date - #222

Open
arpitjain099 wants to merge 1 commit into
FDA:masterfrom
arpitjain099:fix/extract-date-day-range
Open

Clamp the day of month against 31, not 12, in extract_date#222
arpitjain099 wants to merge 1 commit into
FDA:masterfrom
arpitjain099:fix/extract-date-day-range

Conversation

@arpitjain099

Copy link
Copy Markdown

extract_date() in openfda/common.py validates the day of month against 12:

if 0 >= int(month) or 12 < int(month):
  month = "01"
if 0 >= int(day) or 12 < int(day):
  day = "01"

The second bound reads like a copy of the first. The effect is that any day from 13 onward is thrown away and rewritten to the first of the month:

>>> common.extract_date('20121125')
'2012-11-01'
>>> common.extract_date('20121231')
'2012-12-01'

That is roughly 60 percent of all calendar days. AnnotateLabel in openfda/spl/annotate.py feeds the result straight into label['@timestamp'], so SPL effective times land on the wrong day.

Days that overflow a short month do not need this guard either. validate_date() in openfda/spl/fix_date.py already catches day is out of range for month and walks the value back to a real date, which is a better outcome than dropping it to the 1st.

This widens the bound to 31 and leaves the out-of-range case (20121132) behaving exactly as before.

The existing test_extract_date case for day 32 still passes, and I added two cases for valid days past the 12th, which fail on master:

openfda/tests/common_test.py:13: AssertionError
1 failed, 2 passed

With the fix, python3 -m pytest openfda/tests/common_test.py is 3 passed.

extract_date() rejects any day greater than 12 and rewrites it to 01,
so every timestamp whose day of month is 13 or later is silently moved to
the first of the month. 20121125 comes back as 2012-11-01.

The bound looks like a copy of the month check directly above it. Widen it
to 31. Days past the end of a short month are already handled downstream by
validate_date() in openfda/spl/fix_date.py, which walks them back to a real
date instead of discarding them.

Signed-off-by: Arpit Jain <arpitjain099@gmail.com>
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

Successfully merging this pull request may close these issues.

1 participant