Fixes for traider.get_historical_prices, and tests for several data sources. #637
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
The polygon helper now checks for rows with nans and considers them as missing so it can fetch them. Plus some doc comments
I was converting a tz naive index which was throwing off the dates. localizing is the correct way.
renamed the function to clarify its for display purpose
Also refactored it to separate tests for backtesting data sources vs live datasournces
set_pandas_float_display_precision renamed
fixed comment to describe what its doing
this test simulates what the call to get_yesterday_dividends does
that simulates what the call to get_yesterdays_dividend does
…ests pasa this is a weird one. i think my code is working and my tests work. But the original tests fail. I kinda think i have to fix the test, but i spent time trying to do that and i admit defeat. so im leaving my code commented out.
|
Korbit doesn't automatically review large (500+ lines changed) pull requests such as this one. If you want me to review anyway, use |
brettelliot
temporarily deployed
to
unit-tests
GitHub Actions
Inactive
davidlatte
approved these changes
Nov 26, 2024
| # with NANs in cached polygon data, you can try to enable this code and fix the tests. | ||
|
|
||
| # # Find any dates with nan values in the df_all DataFrame | ||
| # missing_dates += df_all[df_all.isnull().all(axis=1)].index.date.tolist() |
There was a problem hiding this comment.
I believe this is the line that doesn't work for you. Completely missing dates are not guarenteed to have an all-empty entry in the DataFrame. This commented out section can be fully removed.
grzesir
approved these changes
Nov 26, 2024
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.
Fixes a bug where I wasn't correctly converting a naive datetime to a tz aware datetime. Also adds a of tests for get_historical_prices. And bumps version of lumiwealth-tradier.
Note
I'm currently writing a description for your pull request. I should be done shortly (<1 minute). Please don't edit the description field until I'm finished, or we may overwrite each other. If I find nothing to write about, I'll delete this message.