fix: source unstable-period ids from the C header (#752)

talib/_common.pxi

@@ -1,3 +1,5 @@
+import warnings
+
 cimport _ta_lib as lib
 from _ta_lib cimport TA_RetCode, TA_FuncUnstId
 
@@ -78,24 +80,73 @@ class MA_Type(object):
 
 MA_Type = MA_Type()
 
-_ta_func_unst_ids = {'NONE': -1}
-for i, name in enumerate([
-            'ADX', 'ADXR', 'ATR', 'CMO', 'DX', 'EMA', 'HT_DCPERIOD',
-            'HT_DCPHASE', 'HT_PHASOR', 'HT_SINE', 'HT_TRENDLINE',
-            'HT_TRENDMODE', 'KAMA', 'MAMA', 'MFI', 'MINUS_DI', 'MINUS_DM',
-            'NATR', 'PLUS_DI', 'PLUS_DM', 'RSI', 'STOCHRSI', 'T3', 'ALL'
-        ]):
-    _ta_func_unst_ids[name] = i
+# Take every id straight from the C header instead of hardcoding a list.
+# Cython resolves each `lib.TA_FUNC_UNST_*` at build time from the ta_defs.h
+# it compiles against, so this one table is correct for every TA-Lib release.
+_ta_func_unst_ids = {
+    # Not sourced from the header: TA_FUNC_UNST_NONE does not exist in newer
+    # ta-lib releases, and -1 is a Python-side sentinel anyway.
+    'NONE': -1,
+    'ADX': lib.TA_FUNC_UNST_ADX,
+    'ATR': lib.TA_FUNC_UNST_ATR,
+    'CMO': lib.TA_FUNC_UNST_CMO,
+    'DX': lib.TA_FUNC_UNST_DX,
+    'EMA': lib.TA_FUNC_UNST_EMA,
+    'HT_DCPERIOD': lib.TA_FUNC_UNST_HT_DCPERIOD,
+    'HT_DCPHASE': lib.TA_FUNC_UNST_HT_DCPHASE,
+    'HT_PHASOR': lib.TA_FUNC_UNST_HT_PHASOR,
+    'HT_SINE': lib.TA_FUNC_UNST_HT_SINE,
+    'HT_TRENDLINE': lib.TA_FUNC_UNST_HT_TRENDLINE,
+    'HT_TRENDMODE': lib.TA_FUNC_UNST_HT_TRENDMODE,
+    'KAMA': lib.TA_FUNC_UNST_KAMA,
+    'MAMA': lib.TA_FUNC_UNST_MAMA,
+    'MINUS_DI': lib.TA_FUNC_UNST_MINUS_DI,
+    'MINUS_DM': lib.TA_FUNC_UNST_MINUS_DM,
+    'NATR': lib.TA_FUNC_UNST_NATR,
+    'PLUS_DI': lib.TA_FUNC_UNST_PLUS_DI,
+    'PLUS_DM': lib.TA_FUNC_UNST_PLUS_DM,
+    'RSI': lib.TA_FUNC_UNST_RSI,
+    'T3': lib.TA_FUNC_UNST_T3,
+    'ALL': lib.TA_FUNC_UNST_ALL,
+}
+
+# Still accepted so existing code keeps running, but they are no longer knobs
+# in TA-Lib C: their enumerators are now TA_FUNC_UNST_UNUSED_*.  They are not
+# aliased to their inner indicator on purpose -- turning a long-standing no-op
+# into a call that changes ADX or RSI for every other function would be a
+# worse surprise than doing nothing.
+_ta_func_unst_retired = {
+    'ADXR': "ADXR has no unstable period of its own; it follows the inner ADX. Set 'ADX' instead.",
+    'MFI': "MFI's unstable period was retired in TA-Lib C.",
+    'STOCHRSI': "STOCHRSI has no unstable period of its own; it follows the inner RSI. Set 'RSI' instead.",
+}
+
+def _ta_func_unst_id(name):
+    if name in _ta_func_unst_retired:
+        warnings.warn(
+            "unstable period '%s' is deprecated and has no effect: %s" % (
+                name, _ta_func_unst_retired[name]),
+            DeprecationWarning, stacklevel=3)
+        return None
+    return _ta_func_unst_ids[name]
 
 def _ta_set_unstable_period(name, period):
     cdef TA_RetCode ret_code
-    cdef TA_FuncUnstId id = _ta_func_unst_ids[name]
+    cdef TA_FuncUnstId id
+    unst_id = _ta_func_unst_id(name)
+    if unst_id is None:
+        return
+    id = unst_id
     ret_code = lib.TA_SetUnstablePeriod(id, period)
     _ta_check_success('TA_SetUnstablePeriod', ret_code)
 
 def _ta_get_unstable_period(name):
     cdef unsigned int period
-    cdef TA_FuncUnstId id = _ta_func_unst_ids[name]
+    cdef TA_FuncUnstId id
+    unst_id = _ta_func_unst_id(name)
+    if unst_id is None:
+        return 0
+    id = unst_id
     period = lib.TA_GetUnstablePeriod(id)
     return period
 

talib/_ta_lib.pxd

@@ -39,31 +39,31 @@ cdef extern from "ta-lib/ta_defs.h":
     const TA_MAType TA_MAType_T3 = 8
 
     ctypedef int TA_FuncUnstId
-    const TA_FuncUnstId TA_FUNC_UNST_ADX = 0
-    const TA_FuncUnstId TA_FUNC_UNST_ADXR = 1
-    const TA_FuncUnstId TA_FUNC_UNST_ATR = 2
-    const TA_FuncUnstId TA_FUNC_UNST_CMO = 3
-    const TA_FuncUnstId TA_FUNC_UNST_DX = 4
-    const TA_FuncUnstId TA_FUNC_UNST_EMA = 5
-    const TA_FuncUnstId TA_FUNC_UNST_HT_DCPERIOD = 6
-    const TA_FuncUnstId TA_FUNC_UNST_HT_DCPHASE = 7
-    const TA_FuncUnstId TA_FUNC_UNST_HD_PHASOR = 8
-    const TA_FuncUnstId TA_FUNC_UNST_HT_SINE = 9
-    const TA_FuncUnstId TA_FUNC_UNST_HT_TRENDLINE = 10
-    const TA_FuncUnstId TA_FUNC_UNST_HT_TRENDMODE = 11
-    const TA_FuncUnstId TA_FUNC_UNST_KAMA = 12
-    const TA_FuncUnstId TA_FUNC_UNST_MAMA = 13
-    const TA_FuncUnstId TA_FUNC_UNST_MFI = 14
-    const TA_FuncUnstId TA_FUNC_UNST_MINUS_DI = 15
-    const TA_FuncUnstId TA_FUNC_UNST_MINUS_DM = 16
-    const TA_FuncUnstId TA_FUNC_UNST_NATR = 17
-    const TA_FuncUnstId TA_FUNC_UNST_PLUS_DI = 18
-    const TA_FuncUnstId TA_FUNC_UNST_PLUS_DM = 19
-    const TA_FuncUnstId TA_FUNC_UNST_RSI = 20
-    const TA_FuncUnstId TA_FUNC_UNST_STOCHRSI = 21
-    const TA_FuncUnstId TA_FUNC_UNST_T3 = 21
-    const TA_FuncUnstId TA_FUNC_UNST_ALL = 22
-    const TA_FuncUnstId TA_FUNC_UNST_NONE = -1
+    # No values here on purpose.  Cython takes the value of a `cdef extern`
+    # const from the C header at build time and ignores anything written on
+    # the right-hand side, so a literal would be decoration that goes stale
+    # without ever failing a build.
+    const TA_FuncUnstId TA_FUNC_UNST_ADX
+    const TA_FuncUnstId TA_FUNC_UNST_ATR
+    const TA_FuncUnstId TA_FUNC_UNST_CMO
+    const TA_FuncUnstId TA_FUNC_UNST_DX
+    const TA_FuncUnstId TA_FUNC_UNST_EMA
+    const TA_FuncUnstId TA_FUNC_UNST_HT_DCPERIOD
+    const TA_FuncUnstId TA_FUNC_UNST_HT_DCPHASE
+    const TA_FuncUnstId TA_FUNC_UNST_HT_PHASOR
+    const TA_FuncUnstId TA_FUNC_UNST_HT_SINE
+    const TA_FuncUnstId TA_FUNC_UNST_HT_TRENDLINE
+    const TA_FuncUnstId TA_FUNC_UNST_HT_TRENDMODE
+    const TA_FuncUnstId TA_FUNC_UNST_KAMA
+    const TA_FuncUnstId TA_FUNC_UNST_MAMA
+    const TA_FuncUnstId TA_FUNC_UNST_MINUS_DI
+    const TA_FuncUnstId TA_FUNC_UNST_MINUS_DM
+    const TA_FuncUnstId TA_FUNC_UNST_NATR
+    const TA_FuncUnstId TA_FUNC_UNST_PLUS_DI
+    const TA_FuncUnstId TA_FUNC_UNST_PLUS_DM
+    const TA_FuncUnstId TA_FUNC_UNST_RSI
+    const TA_FuncUnstId TA_FUNC_UNST_T3
+    const TA_FuncUnstId TA_FUNC_UNST_ALL
 
     ctypedef int TA_RangeType
     const TA_RangeType TA_RangeType_RealBody = 0

tests/test_func.py

@@ -63,6 +63,104 @@ def test_unstable_period():
     talib.set_unstable_period('EMA', 0)
 
 
+# One entry per real unstable-period id in TA-Lib C.  Each id is named after
+# the function it controls, so every case simply calls that same function.
+def _unstable_period_cases():
+    n = 400
+    rs = np.random.RandomState(1)
+    close = np.cumsum(rs.randn(n)) + 100.0
+    high = close + rs.rand(n) + 0.5
+    low = close - rs.rand(n) - 0.5
+    return {
+        'ADX': lambda: func.ADX(high, low, close),
+        'ATR': lambda: func.ATR(high, low, close),
+        'CMO': lambda: func.CMO(close),
+        'DX': lambda: func.DX(high, low, close),
+        'EMA': lambda: func.EMA(close),
+        'HT_DCPERIOD': lambda: func.HT_DCPERIOD(close),
+        'HT_DCPHASE': lambda: func.HT_DCPHASE(close),
+        'HT_PHASOR': lambda: func.HT_PHASOR(close)[0],
+        'HT_SINE': lambda: func.HT_SINE(close)[0],
+        'HT_TRENDLINE': lambda: func.HT_TRENDLINE(close),
+        'HT_TRENDMODE': lambda: func.HT_TRENDMODE(close),
+        'KAMA': lambda: func.KAMA(close),
+        'MAMA': lambda: func.MAMA(close)[0],
+        'MINUS_DI': lambda: func.MINUS_DI(high, low, close),
+        'MINUS_DM': lambda: func.MINUS_DM(high, low),
+        'NATR': lambda: func.NATR(high, low, close),
+        'PLUS_DI': lambda: func.PLUS_DI(high, low, close),
+        'PLUS_DM': lambda: func.PLUS_DM(high, low),
+        'RSI': lambda: func.RSI(close),
+        'T3': lambda: func.T3(close),
+    }
+
+
+UNSTABLE_PERIOD_CASES = _unstable_period_cases()
+
+
+def _leading_unset(result):
+    # TA-Lib writes nothing before its lookback: real outputs stay NaN there,
+    # the one integer output (HT_TRENDMODE) stays 0.
+    valid = ~np.isnan(result) if result.dtype.kind == 'f' else result != 0
+    assert valid.any(), 'no valid output to measure'
+    return int(np.argmax(valid))
+
+
+# NOTE: this has to be a behavioural test.  set_unstable_period() and
+# get_unstable_period() look the id up in the same table, so a wrong id
+# round-trips perfectly -- which is how ta-lib-python shipped a table that
+# pointed 'RSI' at PLUS_DM for two years (issue #752).  Only checking that the
+# setting moves THAT function's own output can catch it.  Please do not
+# "simplify" this into a get/set assertion.
+@pytest.mark.parametrize('name', sorted(UNSTABLE_PERIOD_CASES))
+def test_unstable_period_moves_its_own_function(name):
+    call = UNSTABLE_PERIOD_CASES[name]
+    talib.set_unstable_period(name, 0)
+    unshifted = call()
+    baseline = _leading_unset(unshifted)
+    assert baseline > 0, 'nothing to shift'
+    try:
+        talib.set_unstable_period(name, 5)
+        shifted = call()
+        assert _leading_unset(shifted) == baseline + 5
+        # an unstable period only discards warm-up bars, so whatever both runs
+        # do emit has to be identical -- this pins the shift to a pure delay
+        assert_array_equal(shifted[baseline + 5:], unshifted[baseline + 5:])
+    finally:
+        talib.set_unstable_period(name, 0)
+    assert _leading_unset(call()) == baseline
+
+
+def test_unstable_period_all():
+    talib.set_unstable_period('ALL', 0)
+    baseline = {name: _leading_unset(call())
+                for name, call in UNSTABLE_PERIOD_CASES.items()}
+    try:
+        talib.set_unstable_period('ALL', 3)
+        for name, call in UNSTABLE_PERIOD_CASES.items():
+            assert _leading_unset(call()) == baseline[name] + 3, name
+    finally:
+        talib.set_unstable_period('ALL', 0)
+    for name, call in UNSTABLE_PERIOD_CASES.items():
+        assert _leading_unset(call()) == baseline[name], name
+
+
+# These three are accepted for backwards compatibility only: TA-Lib C retired
+# their unstable-period slots.  They must stay no-ops -- aliasing them to the
+# inner ADX/RSI would give them side effects on every other function.
+@pytest.mark.parametrize('name', ['ADXR', 'MFI', 'STOCHRSI'])
+def test_unstable_period_retired_is_a_warning_and_a_noop(name):
+    talib.set_unstable_period('ALL', 0)
+    before = {n: _leading_unset(call())
+              for n, call in UNSTABLE_PERIOD_CASES.items()}
+    with pytest.deprecated_call():
+        talib.set_unstable_period(name, 5)
+    with pytest.deprecated_call():
+        assert talib.get_unstable_period(name) == 0
+    for n, call in UNSTABLE_PERIOD_CASES.items():
+        assert _leading_unset(call()) == before[n], n
+
+
 def test_compatibility():
     a = np.arange(10, dtype=float)
     talib.set_compatibility(0)