Lightmatter / welkin-health

A Python wrapper of the Welkin Health API
https://welkin.readthedocs.io
Other
4 stars 3 forks source link

Bug/clean date #31

Closed edcohen08 closed 2 years ago

edcohen08 commented 2 years ago

This pr adds a clean date method specifically for the creation of patients with their birthdate but also general use. The clean datetime does not suffice and passing a raw date does not work with the api.

Example of failure:

Screen Shot 2022-08-09 at 6 56 58 PM

Expected behavior is for the date to be converted into a string and the patient successfully created

codecov[bot] commented 2 years ago

Codecov Report

Merging #31 (d959b5a) into develop (ed8d9e3) will decrease coverage by 0.08%. The diff coverage is 87.50%.

:exclamation: Current head d959b5a differs from pull request most recent head 4f4eccf. Consider uploading reports for the commit 4f4eccf to get more accurate results

@@             Coverage Diff             @@
##           develop      #31      +/-   ##
===========================================
- Coverage    93.22%   93.14%   -0.09%     
===========================================
  Files           24       24              
  Lines         1166     1181      +15     
===========================================
+ Hits          1087     1100      +13     
- Misses          79       81       +2     
Impacted Files Coverage Δ
welkin/util.py 86.11% <77.77%> (-3.18%) :arrow_down:
test/test_patients.py 97.77% <100.00%> (+0.40%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

edcohen08 commented 2 years ago

I see what you mean and largely agree but it appears Welkin doesn't even like that. Your example doesn't work to create a patient and neither does this

welkin.Patient(firstName="Devver", lastName="Id", birthDate="2020-12-12T00:00:00.000+04:00").create()

I've only seen

welkin.Patient(firstName="Devver", lastName="Id", birthDate="2020-12-12T00:00:00.000Z").create()

work. I'd gladly be wrong but I think welkin treats it as a utc standardized date.

Edit to add more info.

This is the error:

Screen Shot 2022-08-09 at 9 22 02 PM

Edit 2 because I couldn't help myself.

Enjoy this bit of irony -- note the 'date' above and my date here. one is but one isn't

Screen Shot 2022-08-09 at 9 40 28 PM
samamorgan commented 2 years ago

Ahhh ok. So Welkin is defining a "DATE" object to be a ISO-8601 UTC Zulu string with a zero timestamp. That's silly

I view this as a bug in the Welkin API. A birthdate should be a full timestamp. If a doctor were to tell a patient "Happy Birthday!" but they're a few timezones away, the patient could respond with "Oh, thanks, but that was yesterday". Or maybe it doesn't matter, since the primary use of birth date is probably identity verification.

What do you think of this approach? Keeps us using the same logic for timestamp formatting regardless of input.

def clean_date(date: date) -> str:
    dt = datetime(date.year, date.month, date.day, 0, 0, 0)

    return clean_datetime(dt)

Either way, approved, merge at your leisure.

edcohen08 commented 2 years ago

Sweet, thank you. Opted for your method I much prefer that.