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

add rates calendar #43

Merged
merged 4 commits into from
Nov 7, 2023
Merged

add rates calendar #43

merged 4 commits into from
Nov 7, 2023

Conversation

cctdaniel
Copy link
Contributor

@cctdaniel cctdaniel commented Nov 7, 2023

add rates to calendar and move get_market_hours_clause from data_workflows repo to here as a single source of truth related to calendar logic

)
next_market_close = (
elif dt.date() in NYSE_HOLIDAYS:
next_market_open = get_next_market_open(asset_type, dt)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed this because there was a bug where this would return incorrect answer if the given date is a holiday and the next day is early close holiday

@@ -93,9 +122,15 @@ def test_get_next_market_open():
== format_datetime_to_unix_timestamp(datetime.datetime(2023, 6, 20, 9, 30, 0, tzinfo=NY_TZ))
)

# equity holiday next day early close holiday
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

added new test here to catch the previous bug mentioned

else: # crypto markets never close
return None

return int(next_market_close.timestamp())


def get_market_hours_clause(time_column_name):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this gonna be used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

data workflows for calculating publishers uptime

Copy link
Contributor

@ali-bahjati ali-bahjati left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good but I think the code is getting more and more difficult. We need to find a library to do these.

Also, might be nice to add 2024 holidays.

@cctdaniel
Copy link
Contributor Author

removed get_market_hours_clause because its only used in data_workflows and instead will use the calendar here in data_workflows

@cctdaniel cctdaniel merged commit fd7f08b into main Nov 7, 2023
@cctdaniel cctdaniel deleted the rates branch November 7, 2023 08:41
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.

2 participants