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 doc + types and moves 'special offsets' implementation to XKRX #177

Merged
merged 3 commits into from
Jun 1, 2022

Conversation

maread99
Copy link
Collaborator

@maread99 maread99 commented May 30, 2022

Extends documentation and adds type annotations to ExchangeCalendar "calendar-definition" properties.

Moves 'special offsets' implementation from ExchangeCalendar to XKRXExchangeCalendar (leaving a apply_special_offsets hook into the ExchangeCalendar constructor). The implementation added a fair bit of bulk to ExchangeCalendar although is used only by XKRXExchangeCalendar. Given that no other calendar uses it, and that it's anticipated XKRXExchangeCalendar will move to hardcoded dates anyway (#128), this PR explicitly makes the implementation specific to XKRXExchangeCalendar.

Also

  • a little general linting.
  • tried to add 'black' to dev environment, reverted due to pip conflicts.

Extends documentation and adds type annotations to ExchangeCalendar
"calendar-definition" properties.

Also, adds 'black' to dev environment, as required by
`pre-commit-config.yaml`.

Also, a little general linting.
@maread99 maread99 added the documentation Improvements or additions to documentation label May 30, 2022
Moves 'special offsets' implementation from ExchangeCalendar to XKRX.
Leaves a `apply_special_offsets` hook into the ExchangeCalendar
constructor.
@maread99
Copy link
Collaborator Author

maread99 commented Jun 1, 2022

@gerrymanoim, I'm going to merge this to conclude v3 and get started on v4.

Most of this one is doc and typing improvements. The only part that might be controversial is moving the 'special offsets' implementation from ExchangeCalendar to XKRXExchangeCalendar (as covered by the original comment), although I'm pretty convinced that no other calendar (existing or new) will find a use for it - the way to make ExchangeCalendar more flexible in terms of how session times can be defined would probably be to extend the existing 'calendar definition' properties and methods.

@maread99 maread99 removed the request for review from gerrymanoim June 1, 2022 10:39
@maread99 maread99 merged commit 2576be2 into master Jun 1, 2022
@maread99 maread99 deleted the working-v3 branch June 1, 2022 10:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant