Zigzag indicator implemented from issue #443

[aligheshlaghi97]

Hi again @twopirllc
I removed my previous fork, but I understood that I was not right doing that, cause I added the code to main branch anyway and then send PR to development branch. Or maybe I am wrong again. If so please let me know and make any modifications you want.
Thank you

[twopirllc]

@aligheshlaghi97,

Seems ok. I will take it from here.

Thanks for contributing. 😎
KJ

[aligheshlaghi97]

@twopirllc

That's an honor for me to contribute in this great project, and resolve issue #443 .

If there was any problems with my implementation, please let me know, so I can correct it.

[twopirllc]

Hi @aligheshlaghi97,

Thanks for including an additional indicator to the library. In the future, please make one PR per indicator/bug et al. Independent/atomic PRs helps minimize any any additional edits before the merge into the existing library. 😎

Also other than the detailed comments for alphatrend in in PR #694, is code the same between these two PRs?

Thanks

[aligheshlaghi97]

Hello @twopirllc

Thanks for your time and attention, and I totally understand the problem you declare about independent PRs, and I think I could make a new branch in my local containing the information from main branch of pandas-ta. Then I could have sent every implementation in a unique way. Actually I can do it right now to remove my requests and send them again, or you can separate the two indicators (ZigZag and AlphaTrend) from two different git commits I made.

And to answer your question, the second PR in #694 includes both alphatrend and zigzag indicators, but as mentioned above, they are in two separate git commits.

But on the other hand, if possible, please merge the PRs as my team (and maybe some people who created or contributed in the issue) need these two indicators. And the good point about that is can make more PR as my previous PR is closed and done working with, so I can send request to the target branch easily.

[twopirllc]

@aligheshlaghi97

Actually I can do it right now to remove my requests and send them again, or you can separate the two indicators (ZigZag and AlphaTrend) from two different git commits I made.

And to answer your question, the second PR in #694 includes both alphatrend and zigzag indicators, but as mentioned above, they are in two separate git commits.

No worries. Thanks for clarifying. I'll figure it out.

But on the other hand, if possible, please merge the PRs as my team (and maybe some people who created or contributed in the issue) need these two indicators.

I will look into merging it after I merge PR #623 since it has been on hold for several months due to my limited free time this past year. 😑

I appreciate your/team interest to contribute to this library. It really helps! 😎

KJ

[aligheshlaghi97]

@twopirllc You're very welcome, and thank you for your considerations despite your limited time.

And also I would like to say that I have more free times these days and so if you had any feedback, I would love to apply it into the code asap.

[KilianB]

@aligheshlaghi97 I attempted to use your code outside of pandas-ta but run into an error on a bigger time series.

Reproducible example can be found here: https://colab.research.google.com/drive/1biUYxTmtgP0a76yBrdqbZuhKpmZy9tSV?usp=sharing#scrollTo=FkNAt_zoUCNM

Can you please help me understand if this is just an issue with my dataset or if there is a bug in your implementation?

[aligheshlaghi97]

Thanks @KilianB for mentioning the problem you faced. And you are totally right about the error. It would be great if you please more clarify about the dataset you use. However I tried to resolve the problem and which is seemingly handled by adding these two lines of codes to previous swing_seq_catch function of previous code. I know it might not be the best solution, but it solves the problem for now.

new_s = new_s.dropna()
new_s = new_s[~new_s.index.duplicated(keep='first')]

The whole swing_seq_catch will be like this:

  def swing_seq_catch(swing_data):
      """
      This function should get swings data and will return the swings,
      with one column added named new_swing_type which will determine if there is a change in swing type,
      e.g. from high to low or vice versa.
      """
      new_swings = swing_data[(swing_data.swing_type.shift(1) == swing_data.swing_type) |
                              (swing_data.swing_type.shift(-1) == swing_data.swing_type)]
      if len(new_swings) > 0:
          swing_change_idx = (swing_data.swing_type.shift(1) != swing_data.swing_type).cumsum()
          new_s = new_swings['swing_type'].astype(str) + swing_change_idx.astype('str')
          new_s = new_s.dropna()
          new_s = new_s[~new_s.index.duplicated(keep='first')]
          new_swings.insert(2, "new_swing_type", new_s, True)
      return new_swings

Also it worth mentioning that the problem was raised due to generating multiple duplicate values inside swings.
If this was not the solution for you, please kindly let me know. And also if you find any other problems, feel free to contact.

I will send a new PR if @twopirllc agrees with this change.

Best Regards,
Ali Gheshlaghi

[KilianB]

@aligheshlaghi97 sure, thanks for creating a python version of this useful indicator. The data I am using has been reconstructed from TickData downloaded from Dukascopy and in my case is reconstructed into 15 minute bars. If you take a look at the Colab notebook you should also have access to the data file. The link should be publicly accessible.

My goal is to use the zigzag indicator as a pre filtering step to extract useful test data for further ml models, thus I am perfectly fine if it repaints and actually would prefer it to be identical to the version found inside the mt4 or mt5 instances.

I just rand your modified code and it is executing. Great :-), but there is still some unexpected behavior: the arguments retrace and last_extreme are unused in the code.

The resulting zig zag somehow misses the extreme points:

range = 50000
deviation = 0.7
zz = zigzag(high = eur_usd['High'][:range],low = eur_usd['Low'][:range],price_deviation=deviation)
plt.plot(eur_usd['Close'][:range])
plt.plot(zz[1]["swing_value"])

Increasing the pivot leg value solves this misbehavior, but I don't quit understand what this value is for?
Now I understand that price_leg is the driving factor, I somehow focused on the deviation instead. That one is on me.

[aligheshlaghi97]

@KilianB Thanks for your great job testing and utilizing this function. I totally understand that there might be some problems calculating this indicator, and thanks for sharing the pictures, but can you please clarify the parameters you passed resulting in these two pictures?

Also talking about retrace and last_extreme, I tried to use the definition of this indicator on TradingView, but I didn't know how to utilize those two variables. If you can help me how should I utilize them, it would be great.

And my suggestion is to see the MQL code for ZigZag indicator, so that we might have better insight over what is happening exactly regarding this indicator.

Best Regards,
Ali Gheshlaghi