facioquo / stock-indicators-python

Stock Indicators for Python. Maintained by @LeeDongGeon1996
https://python.StockIndicators.dev
Apache License 2.0
220 stars 37 forks source link

chore: Bump DLL from 1.23.1 to 2.4.0 #264

Closed LeeDongGeon1996 closed 1 year ago

LeeDongGeon1996 commented 1 year ago

Description

Checklist

LeeDongGeon1996 commented 1 year ago

@DaveSkender I think that there's no way to calc SMA with other price value (not close value) by using v2.3.0(C#) api, because we currently don't have chaining or BasicData feature. 🤔

Maybe.. do we need to add BasicData feature for internal use? --> it works.

results = CsIndicator.GetBaseQuote[Quote](CsList(Quote, quotes), candle_part.cs_value)
results = CsIndicator.GetSma(results, lookback_periods)

But, in this case, the error shows up when "empty quotes":

System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index') E at Skender.Stock.Indicators.Indicator.ToResultTuple(IEnumerable`1 basicData)

E at Skender.Stock.Indicators.Indicator.GetSma(IEnumerable`1 results, Int32 lookbackPeriods)

So, it needs conditional like below:

quotes = CsList(Quote, quotes)
if quotes:
    quotes = CsIndicator.GetBaseQuote[Quote](quotes, candle_part.cs_value)
    results = CsIndicator.GetSma(quotes, lookback_periods)
else:
    results = CsIndicator.GetSma[Quote](quotes, lookback_periods)

Looks not pretty.. Is it because ToResultTuple() cannot accept empty list?


FYI, pythonnet still cannot support extension method in my guess. so calling like this is not possible:

results = CsIndicator.GetBaseQuote[Quote](CsList(Quote, quotes), candle_part.cs_value).GetSma(lookback_periods)
DaveSkender commented 1 year ago

You're really getting into the chaining features going that route. It might be simpler to just stick with the basic API, then come back to adding the chaining parts.

https://github.com/DaveSkender/Stock.Indicators/blob/c861415538c93d04626a3c8df618cfadc153deeb/src/s-z/Sma/Sma.Api.cs#L9-L14

LeeDongGeon1996 commented 1 year ago

Yeah, the problem comes from that. The basic API of SMA can calculate from only close values. Is it intentional? I'd like to provide options so that users can select which OHLCV used for calculating.

DaveSkender commented 1 year ago

Yeah, the problem comes from that. The basic API of SMA can calculate from only close values. Is it intentional? I'd like to provide options so that users can select which OHLCV used for calculating.

Ah, I forgot we had that parameter before, on both SMA and EMA. I removed it because with the chainable Use() it wouldn’t be needed. Can you think of a simple way to keep the old wrapper API with the new DLL for these two? Maybe put the Use() or ToBasicTuple() inside the wrapper API?

LeeDongGeon1996 commented 1 year ago

It works well with all of them. I'm just curious. why it does not work with GetBaseQuote()? It shows the error like the below when testing with noquote. As I know, GetBaseQuote(), Use(), ToBasicTuple have same return type, isn't it?

E       System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection. (Parameter 'index')
E          at Skender.Stock.Indicators.Indicator.ToResultTuple(IEnumerable`1 basicData)
E          at Skender.Stock.Indicators.Indicator.GetSma(IEnumerable`1 results, Int32 lookbackPeriods)
LeeDongGeon1996 commented 1 year ago

@DaveSkender Just for double-check, are test values the only changes in PRS and Renko?

DaveSkender commented 1 year ago

Were you able to check that the breaking changes listed in https://github.com/DaveSkender/Stock.Indicators/pull/802 are considered here? Most easily missed is the swapping of parameter locations for Beta and PRS.

LeeDongGeon1996 commented 1 year ago

Thank you for letting me know. Breaking changes on v2 dll

Also:

LeeDongGeon1996 commented 1 year ago

I think it's better to add related documentation right before releasing in another PR :)

DaveSkender commented 1 year ago

I think it's better to add related documentation right before releasing it in another PR :)

Sure, documentation can be done separately in another feature branch. Since we don't time our doc vs package releases, that makes sense. I'd recommend having an open "update-docs" branch to increment alongside code changes, but that's entirely up to you.

LGTM, thanks đź‘Ť