-
Notifications
You must be signed in to change notification settings - Fork 176
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
Schedule 'force_special_times=True' performance optimization #358
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I see you closed this without merging, is there a reason for that? |
Being a Hardware Engineer I'm pretty new to Github. What I pull-requested
was an isolated update, but I'm still making other updates to the repo to
add a couple things. Because of that, I just committed some partial changes
to my own Dev Branch. I didn't realise that those automatically got roped
in with the already open PR. Once I'm done adding stuff, and everything is
in a finalized state then I was going to reopen the PR.
…On Sat, Dec 14, 2024 at 11:59 AM rsheftel ***@***.***> wrote:
I see you closed this without merging, is there a reason for that?
—
Reply to this email directly, view it on GitHub
<#358 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BFIBUJ2K6SRLUOQAYRNVP6T2FRWXRAVCNFSM6AAAAABTOL7VS6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNBTGIYTCNJWHE>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
Sounds great, thanks!
…On Sat, Dec 14, 2024, at 1:06 PM, Bryce Hawk wrote:
Being a Hardware Engineer I'm pretty new to Github. What I pull-requested
was an isolated update, but I'm still making other updates to the repo to
add a couple things. Because of that, I just committed some partial changes
to my own Dev Branch. I didn't realise that those automatically got roped
in with the already open PR. Once I'm done adding stuff, and everything is
in a finalized state then I was going to reopen the PR.
On Sat, Dec 14, 2024 at 11:59 AM rsheftel ***@***.***> wrote:
> I see you closed this without merging, is there a reason for that?
>
> —
> Reply to this email directly, view it on GitHub
> <#358 (comment)>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/BFIBUJ2K6SRLUOQAYRNVP6T2FRWXRAVCNFSM6AAAAABTOL7VS6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNBTGIYTCNJWHE>
> .
> You are receiving this because you modified the open/close state.Message
> ID: ***@***.***>
>
—
Reply to this email directly, view it on GitHub <#358 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACYBJIY265SABHQYNM7LAGL2FRXQTAVCNFSM6AAAAABTOL7VS6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNBTGIZDIMZUGU>.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
Changed ValueError on overlapping session to a warning and added as system for escalating date_range warnings to errors. Added/adjusted relevant tests
- Docstring updated - Updated and Expanded Tests
This is great, thanks for your work. |
No problem! There are still a few other (hopefully smaller) things along the same vein i might add here soon. They're things I personally could use, but fit well in the scope of your library so I figured I'd add them as contributions instead. |
Please feel free to add anything you think may be useful.
…On Sun, Jan 5, 2025, at 7:03 PM, Bryce Hawk wrote:
No problem! There are still a few other (hopefully smaller) things along the same vein i might add here soon. They're things I personally could use, but fit well in the scope of your library so I figured I'd add them as contributions instead.
—
Reply to this email directly, view it on GitHub <#358 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACYBJI6TY4YOT3D5QEPZY632JHB4NAVCNFSM6AAAAABTOL7VS6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKNZRHAYDANRSGQ>.
You are receiving this because you modified the open/close state.Message ID: ***@***.***>
|
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When force_special_times is True, the special_times are applied to other columns by editing an intermediate pandas Series Object. This was changed so that the code now edits an intermediate numpy array which is slightly more performant.
This change is most noticeable when generating longer schedules, or calendars that have re-occurring special times (such as XTAE's early sunday close)
Average NYSE Schedule Generation Time [2000-01-01 -> 2025-01-01]
* These numbers should, in theory, be identical to their values in the last row. Random variations in execution time are causing the small discrepancies