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

DonchianChannel problem #133

Open
yomoko opened this issue Mar 26, 2020 · 11 comments
Open

DonchianChannel problem #133

yomoko opened this issue Mar 26, 2020 · 11 comments
Assignees

Comments

@yomoko
Copy link

yomoko commented Mar 26, 2020

Hi, I think there's something wrong with the donchian channel method.

  1. It's one period off. Eg, If you use a broker chart (with the same D-Channel) to compare with the python calculated result, it's off. I have to use shift(1) to make the date sync with the correct result.
  2. Even if we specify n=100 and fillna=False, there's no na value (the first n values) in the result
  3. For hband, we actually have to use ta.volatility.DonchianChannel(high) and lband, ta.volatility.DonchianChannel(low), which is two different operation. But in the documentation and they way how it's structured, it says to use 'close' column, which is very mis-leading and wrong.

Why is that? Thanks.

@bukosabino bukosabino self-assigned this Mar 29, 2020
@bukosabino
Copy link
Owner

Hi @yomoko ,

Thank you for your advice. I will do some tests to check the results of the Donchian Channel asap.

Best,
Dario

@yomoko
Copy link
Author

yomoko commented Mar 29, 2020

@bukosabino
For pt 1, the definition should be the past n-period high/low. NOT current n-period. If it uses current n-period, the indicator will never return 1 (Because the price can never close above and below the channel).

@matteosantama
Copy link

I'm not sure I agree with your interpretation of the indicators for pt 1). The indicator you're suggesting is simply whether we see an N day high or low

@bukosabino
Copy link
Owner

Hi @yomoko ,

I have worked on the DonchianChannel indicator. I have added some useful functions and fix some minor details. By answering your comments:

  1. I have checked the Donchian Channel implementation, and I think it is correct. I don't agree with your interpretation of the indicator. I think we need to include the last n-periods (including the current close value). Can you share with me what broker chart implements the indicator using your way? Of course, I am open to discuss this topic.

I have developed some test to check the results, and they match with this Google Sheet doc: https://docs.google.com/spreadsheets/d/17JWWsxSiAb24BLzncUpccc8hg-03QjVWVXmoRCJ2lME/edit#gid=0

Please, check it and let me know if there is something wrong.

  1. I have fixed the fillna statements. Now, if you set the fillna=False you will get n NaN values.

  2. You don't need the low or high series. The way to use the indicator would be:

indicator = ta.volatility.DonchianChannel(close=df['close'], n=20, fillna=False)
df['d_channel_high'] = indicator.donchian_channel_hband()
df['d_channel_low'] = indicator.donchian_channel_lband()
df['d_channel_middle'] = indicator.donchian_channel_mband()
df['d_channel_width'] = indicator.donchian_channel_wband()
df['d_channel_percentage'] = indicator.donchian_channel_pband()
df['d_channel_high_indicator'] = indicator.donchian_channel_hband_indicator()
df['d_channel_low_indicator'] = indicator.donchian_channel_lband_indicator()

To use all of this, you need to update the last version (0.5.21) of the library:

pip install -U ta

or

pip install ta==0.5.21

Best,
Dario

@yomoko
Copy link
Author

yomoko commented May 4, 2020

@bukosabino
Well... since you have point 3 wrong in the first place (lowest low and highest high of past n-period, not highest close and lowest close of past n-period), you don't see any problem with point 1.

@bukosabino
Copy link
Owner

Hi @yomoko ,

You are right, I should calc the highest high and the lowest low. Let me try a new proposal.

Best,
Dario

@yomoko
Copy link
Author

yomoko commented May 4, 2020

@bukosabino
Great. Then you will see that if you do so as point 1, nothing will actually close ABOVE or BELOW the channel, which means the following will only return 0:
image

Because the max you get is close = hband(current close = current high) and the min close = lband(current close = current low) if current period is being used. But the definition is actually LAST n-period. A lot of charting programs out that are actually wrong. Eg. tradingview & metatrader

bukosabino added a commit that referenced this issue May 4, 2020
bukosabino added a commit that referenced this issue May 4, 2020
fixing bug donchian channels #133
@bukosabino
Copy link
Owner

I have fixed the indicator:

  • I use the highest high to calc the upper channel.
  • I use the lowest low to calc the lower channel.
  • I delete the donchian channel_lband_indicator and donchian_channel_hband_indicator. They don't make sense.
  • I add the offset input parameter. It is useful to calc the indicator with n periods delay.

In your case, if you want the last n-periods, you should call the indicator:

from ta.volatility.DonchianChannel


indicator = ta.volatility.DonchianChannel(high=df['high'], low=df['low'], close=df['close'], n=20, offset=1, fillna=False)
df['d_channel_high'] = indicator.donchian_channel_hband()
df['d_channel_low'] = indicator.donchian_channel_lband()
df['d_channel_middle'] = indicator.donchian_channel_mband()
df['d_channel_width'] = indicator.donchian_channel_wband()
df['d_channel_percentage'] = indicator.donchian_channel_pband()

The results of the tests matches with these documents:

  1. Using offset = 0 https://docs.google.com/spreadsheets/d/17JWWsxSiAb24BLzncUpccc8hg-03QjVWVXmoRCJ2lME/edit?usp=sharing

  2. Using offset = 1 https://docs.google.com/spreadsheets/d/1BnYRoP9cjy-SP9TKnkTwJtUhUuZeDQbzXMQHNsNPSC4/edit?usp=sharing

It is available in the last library version (0.5.22).

pip install -U ta

or

pip install ta==0.5.22

Thank you for your help. Please, let me know if you see anything wrong!

Best,
Dario

@yomoko
Copy link
Author

yomoko commented May 4, 2020

@bukosabino
It looks good. But still.. I can't think of a reason why offset=0 is correct. But since there's an option to change it, I guess no point to argue if it should be 1-period delayed or not.

@bukosabino
Copy link
Owner

Hi @yomoko ,

Could you provide me some link/information about the offset should be 0 or 1?

Best,
Dario

@yomoko
Copy link
Author

yomoko commented May 5, 2020

@bukosabino
https://www.incrediblecharts.com/indicators/donchian_channels.php
image
As I said, a lot of big guys have it wrong for whatever reason...

There's nothing wrong with your deleted channel band indicator, but since you have your previous point 1 wrong, that's what made the indicator doesn't make sense. I have said that numerous times already.

And if you still don't get it, as I said in the previous reply, forget it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants