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

Fix multiday div-adjust ; Tidy adjust args #1434

Draft
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

ValueRaider
Copy link
Collaborator

@ValueRaider ValueRaider commented Feb 19, 2023

Dividend adjustment of multi-day intervals is broken - always was - and in some cases the 'Adj Close' returned by Yahoo is wrong. Details & planned solution in Issue #1273. Most code changes are in Ticker._get_div_adjusted_multiday_prices().

Make some other changes:

Changed Ticker.history(auto_adjust=...) default to False, finally matching yf.download behaviour. Deleted back_adjust argument, it just duplicated auto_adjust.

Renamed auto_adjust -> div_adjust. auto_adjust still works but prints a "Deprecated" message, when #1423 is complete can limit prints to one time.

@RudyNL Any thoughts? #1333

Fix multiday div-adjustment, Yahoo's 'Adj Close' is bad ; Rename 'auto_adjust'->'div_adjust', drop 'back_adjust'
Copy link

@paulmcq paulmcq left a comment

Choose a reason for hiding this comment

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

Line yfinance/base.py 1470:
if "period" in hist_args and hist_args["period"] is not None:
might be more legible as:
if hist_args.get("period", None) is not None:

@ValueRaider
Copy link
Collaborator Author

ValueRaider commented Feb 19, 2023

@paulmcq You can review code in "Files changed" tab -> click '+' button by code

ohlcv = ["Open", "High", "Low", "Close", "Volume"]

df_unadj = None
if "period" in hist_args and hist_args["period"] is not None:
Copy link

@paulmcq paulmcq Feb 19, 2023

Choose a reason for hiding this comment

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

might be more legible as:
if hist_args.get("period", None) is not None:

Copy link
Contributor

@ricardoprins ricardoprins left a comment

Choose a reason for hiding this comment

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

Here's what I got from confronting the data from #1273:

import yfinance as yf


ticker = "i3e.l"
data = yf.Ticker(ticker)
start = "2022-11-07"
end = "2022-11-14"

df_yr = data.history(start=start, end=end, interval="1d")
print(df_yr[['Close', 'Adj Close', 'Dividends']])

df_yr = data.history(start=start, end=end, interval="1wk")
print(df_yr[['Close', 'Adj Close', 'Dividends']])

Results:

NOTICE: yfinance.Ticker::history(): Be aware that dividend-adjustment is now default disabled, default used to be enabled
                               Close  Adj Close  Dividends
Date                                                      
2022-11-07 00:00:00+00:00  24.600000  23.223310     0.0000
2022-11-08 00:00:00+00:00  24.500000  23.128904     0.0000
2022-11-09 00:00:00+00:00  23.549999  22.232069     0.0000
2022-11-10 00:00:00+00:00  23.500000  22.319923     0.1425
2022-11-11 00:00:00+00:00  23.500000  22.319923     0.0000
                           Close  Adj Close  Dividends
Date                                                  
2022-11-07 00:00:00+00:00   23.5  22.184866     0.1425

Process finished with exit code 0

Just a quick note: I had to edit line 226 in base.py to make it work, matching what we have in dev:

dt_now = _pd.Timestamp.utcnow()

It seems to me that the modifications are doing what was expected of them unless I'm missing something here.

@ValueRaider
Copy link
Collaborator Author

I tagged review for discussion - should yfinance do this?

@ricardoprins
Copy link
Contributor

I think it could indicate the distortion and give the user the option to enable/disable the correction via flag.

@ValueRaider
Copy link
Collaborator Author

I could link the 'dividend adjustment fix' to argument repair=True. Which is the bulk of this PR.

@ValueRaider ValueRaider marked this pull request as draft October 19, 2023 18:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants