FEAT/Calculate business days elapsed based on two inputs#31
FEAT/Calculate business days elapsed based on two inputs#31cris-seaton wants to merge 12 commits intomasterfrom
Conversation
|
remaining: documentation for create_udf_business_hours macro. |
alex-m01
left a comment
alex-m01
left a comment
There was a problem hiding this comment.
hi @cris-seaton ! I've left a few comments and suggestions. If you have any questions, please let me know.
cris-seaton
changed the title
| # weekend, therefore return 0 via max(0,business_hours). | ||
|
|
||
| while start_datetime.date() in holiday_list or start_datetime.weekday() in [5,6]: | ||
| start_datetime = start_datetime.replace(hour=opening_hour,minute=0,second=0) + timedelta(days=1) |
There was a problem hiding this comment.
cannot utilize L31 or L32 here because these are different context, this while loop turns start_datetime in first_day_OPENED (i.e. if start occurs on holiday or weekend, iterate forward to 9am first available non-holiday or weekday.
macros/udfs/business_hours.sql
Outdated
| # When start_datetime is iterated beyond end_datetime, this indicates entirety of hours occuring during holidays or | ||
| # weekend, therefore return 0 via max(0,business_hours). | ||
|
|
||
| while start_datetime.date() in holiday_list or start_datetime.weekday() in [5,6]: |
There was a problem hiding this comment.
After your review, going to talk to Peter about practicality on omitting weekdays. we could make it a boolean tag in the call, (i.e.
udf_business_hours(start_datetime, end_datetime, country, omit_weekends[Yes|No]) but it will complicate the code with more conditional if statements for the while loop L48 and the if statement L57
|
@alex-m01 set for re-review. |
JFrackson
left a comment
JFrackson
left a comment
There was a problem hiding this comment.
This looks great. Is there a standard that you've worked on to help test these UDFs? When they're wrapped up in SQL I can imagine it's more challenging. Is there a simple way of defining these python functions in Python files that can then be tested with pytest?
3 participants
addressing the click-up task: https://app.clickup.com/t/863gevf8v