Repository navigation
fix(data): accept datetime bounds in the PIT P operator - #2376
Open
Divyansh Barodiya (Divyansh151005) wants to merge 1 commit into
Open
Divyansh Barodiya (Divyansh151005) wants to merge 1 commit into
Divyansh Barodiya (Divyansh151005) wants to merge 1 commit into
Conversation
time2idx=False passes query datetimes into P. Subtracting those timestamps produces a Timedelta, so building the result length raised TypeError. Datetime bounds are now aligned to the calendar, and that path keeps a datetime index.
Author
|
@microsoft-github-policy-service agree |
This file contains hidden or 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
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.
Description
Pnow accepts the datetime bounds passed byLocalExpressionProviderwhentime2idx=False.P._load_internalbuilds its result length withend_index - start_index + 1and then walks integer calendar positions. Withtime2idx=Truethose bounds are calendar positions. Withtime2idx=Falsethe provider passes the query datetimes through.Timestamp - Timestampis aTimedelta, andTimedelta + 1raises:Datetime bounds are aligned with
Cal.locate_index. That path returns a series indexed by the aligned trading days, soseries.loc[start_time:end_time]in the provider still selects the requested range. Integer queries still return aRangeIndex. A future period reference is still rejected, and missing period data still returns an empty series.Motivation and Context
Fixes #1551
P($$roewa_q)is the supported way to read a point-in-time feature on each trading day.time2idx=Falseis the expression-provider mode for datetime-indexed data, andPwas the operator that crashed on those bounds.How Has This Been Tested?
pytest qlib/tests/test_all_pipeline.pyunder upper directory ofqlib.test_all_pipeline.pytrains a CSI300 LightGBM workflow and needs the market dataset. These tests stub the period feature and the calendar. They do not read PIT files or market data.flake8on the changed Python files, with the ignore list from the developer guide, reported no issues.black -l 120left the files unchanged.Screenshots of Test Results (if appropriate):
tests/ops/test_pit_operator.py, 4 passed.Types of changes