Skip to content

Commit 66422a0

Browse files
DaveSkenderDaveSkender
authored
fix: Correct inverted ratio returned by obsolete GetPrs (#2210)
1 parent ee00eed commit 66422a0

3 files changed

Lines changed: 242 additions & 11 deletions

File tree

‎docs/migration/v3.md‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,28 @@ The old `Quote`, `IQuote`, `PeriodSize`, `IReusableResult`, and `BasicData` name
5858
- **`ToTupleCollection()` utility**: Deprecated
5959
- **`ToCollection()` utility**: Deprecated
6060

61+
### Corrections to obsolete v2 shims
62+
63+
Defects in the `[Obsolete]` compatibility shims, shipped in `3.0.0` and **corrected in
64+
`3.0.1`**. Each changes **returned values** for code still calling the v2 method names.
65+
They are worth correcting even though the shims themselves are removed in `3.1`: `3.0.x`
66+
is the version you are told to land on before upgrading further, so it has to be right.
67+
68+
These version as bug fixes rather than breaking changes because the shipped behavior
69+
never matched its own documentation — see the correctness principles on [correcting a
70+
defect versus changing intended behavior](https://github.com/facioquo/stock-indicators-dotnet/blob/main/docs/PRINCIPLES.md).
71+
72+
- **`GetPrs()` returned the inverted ratio.** In `3.0.0` the shim passed its two series
73+
in the wrong order, so it returned `base / eval` — the reciprocal of the ratio
74+
[documented for PRS](/indicators/prs). As of `3.0.1` it returns `eval / base`, matching
75+
`ToPrs()` and pre-`3.0.0` behavior. **If you called `GetPrs()` on `3.0.x`, stored values and any
76+
thresholds tuned against them must be revalidated; new values are the reciprocal of
77+
old ones.** Callers of `ToPrs()` were never affected.
78+
- **`GetPrs()` with no `lookbackPeriods` threw.** The unspecified lookback was mapped to
79+
`0`, which validation rejects, so the shim's own default raised
80+
`ArgumentOutOfRangeException`. As of `3.0.1` it computes with a null `PrsPercent`, as
81+
it did before `3.0.0`.
82+
6183
### Other changes
6284

6385
- **Indicator method parameters**: v2 generic signatures like `GetSma<TQuote>(this IEnumerable<TQuote>)` are now interface-typed, like `ToSma(this IReadOnlyList<IReusable>)` and `ToFractal(this IReadOnlyList<IBar>)`. Concrete lists (e.g. `List<Bar>`) convert automatically; your own generic wrapper methods need a `class` constraint (see [Step 4](#step-4-update-generic-extension-methods))

‎src/Obsolete.V3.Indicators.cs‎

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -680,18 +680,22 @@ public static IEnumerable<PmoResult> GetPmo(
680680
public static IEnumerable<PrsResult> GetPrs(
681681
this IEnumerable<IBar> quotesEval,
682682
IEnumerable<IBar> quotesBase, int? lookbackPeriods = null)
683-
=> quotesBase.ToSortedList()
684-
.ToPrs(quotesEval.ToSortedList(), lookbackPeriods ?? 0);
683+
// an unspecified lookback means "compute no PrsPercent", which is the
684+
// two-argument overload -- not lookbackPeriods 0, which validation rejects
685+
=> lookbackPeriods is int periods
686+
? quotesEval.ToSortedList().ToPrs(quotesBase.ToSortedList(), periods)
687+
: quotesEval.ToSortedList().ToPrs(quotesBase.ToSortedList());
685688

686689
[ExcludeFromCodeCoverage]
687690
[Obsolete("Use a chained `results.ToSma(smaPeriods)` for moving averages.", true)]
688691
public static IEnumerable<PrsResult> GetPrs(
689692
this IEnumerable<IBar> quotesEval,
690693
IEnumerable<IBar> quotesBase, int? lookbackPeriods, int? smaPeriods = null)
691-
=> quotesEval
692-
.ToSortedList()
693-
.Use(CandlePart.Close)
694-
.ToPrs(quotesBase.ToSortedList().Use(CandlePart.Close), lookbackPeriods ?? 0);
694+
=> lookbackPeriods is int periods
695+
? quotesEval.ToSortedList().Use(CandlePart.Close)
696+
.ToPrs(quotesBase.ToSortedList().Use(CandlePart.Close), periods)
697+
: quotesEval.ToSortedList().Use(CandlePart.Close)
698+
.ToPrs(quotesBase.ToSortedList().Use(CandlePart.Close));
695699

696700
[ExcludeFromCodeCoverage]
697701
[Obsolete("Use 'ToPrs(..)' method. Tuple arguments were removed.", false)]
@@ -700,11 +704,21 @@ public static IEnumerable<PrsResult> GetPrs(
700704
IEnumerable<(DateTime d, double v)> tupleBase,
701705
int lookbackPeriods = 0,
702706
int smaPeriods = 0)
703-
=> tupleEval
704-
.Select(static t => new TimeValue(t.d, t.v)).ToSortedList()
705-
.ToPrs(tupleBase
706-
.Select(static t => new TimeValue(t.d, t.v)).ToSortedList(),
707-
lookbackPeriods);
707+
// v2 declared this int? and treated null as "no PrsPercent"; the 3.0.0 shim
708+
// narrowed it to int = 0, so 0 is the only marker left for "unspecified".
709+
// Restoring the default therefore also accepts an explicit 0, which v2 rejected
710+
// -- the alternative, changing the signature back, would break already-compiled
711+
// callers rather than fix them.
712+
=> lookbackPeriods == 0
713+
? tupleEval
714+
.Select(static t => new TimeValue(t.d, t.v)).ToSortedList()
715+
.ToPrs(tupleBase
716+
.Select(static t => new TimeValue(t.d, t.v)).ToSortedList())
717+
: tupleEval
718+
.Select(static t => new TimeValue(t.d, t.v)).ToSortedList()
719+
.ToPrs(tupleBase
720+
.Select(static t => new TimeValue(t.d, t.v)).ToSortedList(),
721+
lookbackPeriods);
708722

709723
[ExcludeFromCodeCoverage]
710724
[Obsolete("Rename `GetPvo(..)` to `ToPvo(..)`", false)]

‎tests/Library/Indicators/k-q/Prs/PrsObsoleteShimTests.cs‎

Lines changed: 195 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,195 @@
1+
using System.Reflection;
2+
3+
namespace StaticSeries;
4+
5+
/// <summary>
6+
/// Tests for the obsolete v3 <c>GetPrs</c> shims, which must return what the pre-3.0.0
7+
/// API returned.
8+
/// </summary>
9+
/// <remarks>
10+
/// The shim exists only to keep v2 code working, so "matches <c>ToPrs</c>" is its whole
11+
/// contract. Two defects shipped in 3.0.0 and survived because nothing here called it:
12+
/// the evaluated and base series were passed in the wrong order, so every result was the
13+
/// reciprocal of the ratio; and an unspecified lookback mapped to <c>0</c>, which
14+
/// validation rejects, so the documented default threw instead of computing.
15+
/// </remarks>
16+
[TestClass]
17+
public class PrsObsoleteShimTests : TestBaseWithPrecision
18+
{
19+
[TestMethod]
20+
public void GetPrsMatchesToPrsRatioDirection()
21+
{
22+
const int lookbackPeriods = 20;
23+
24+
#pragma warning disable CS0618 // exercising the obsolete shim is the point
25+
IReadOnlyList<PrsResult> shim
26+
= Bars.GetPrs(OtherBars, lookbackPeriods).ToList();
27+
#pragma warning restore CS0618
28+
29+
IReadOnlyList<PrsResult> expected
30+
= ((IReadOnlyList<IReusable>)Bars).ToPrs(OtherBars, lookbackPeriods);
31+
32+
shim.Select(static r => r.Prs).Should().Equal(expected.Select(static r => r.Prs),
33+
"GetPrs(quotesEval, quotesBase) evaluates the first series against the second, as it did before 3.0.0");
34+
35+
shim.Select(static r => r.PrsPercent).Should().Equal(expected.Select(static r => r.PrsPercent));
36+
}
37+
38+
[TestMethod]
39+
public void GetPrsIsNotTheInvertedRatio()
40+
{
41+
// guards the specific 3.0.0 regression: the arguments were swapped, so every
42+
// value came back as the reciprocal
43+
const int lookbackPeriods = 20;
44+
45+
#pragma warning disable CS0618
46+
List<PrsResult> shimResults = Bars.GetPrs(OtherBars, lookbackPeriods).ToList();
47+
double shim = shimResults[^1].Prs!.Value;
48+
#pragma warning restore CS0618
49+
50+
IReadOnlyList<PrsResult> invertedResults
51+
= ((IReadOnlyList<IReusable>)OtherBars).ToPrs(Bars, lookbackPeriods);
52+
double inverted = invertedResults[^1].Prs!.Value;
53+
54+
shim.Should().BeApproximately(1 / inverted, Money6);
55+
}
56+
57+
[TestMethod]
58+
public void GetPrsWithUnspecifiedLookbackComputesWithoutPercent()
59+
{
60+
// the shim's own default: v2 treated it as "no PrsPercent", not as an error
61+
#pragma warning disable CS0618
62+
IReadOnlyList<PrsResult> shim = Bars.GetPrs(OtherBars).ToList();
63+
#pragma warning restore CS0618
64+
65+
IReadOnlyList<PrsResult> expected
66+
= ((IReadOnlyList<IReusable>)Bars).ToPrs(OtherBars);
67+
68+
shim.Should().HaveCount(expected.Count);
69+
shim.Select(static r => r.Prs).Should().Equal(expected.Select(static r => r.Prs));
70+
shim.Should().OnlyContain(static r => r.PrsPercent == null);
71+
}
72+
73+
[TestMethod]
74+
public void GetPrsFromTuplesMatchesToPrs()
75+
{
76+
IEnumerable<(DateTime d, double v)> evalTuples
77+
= Bars.Select(static b => (b.Timestamp, (double)b.Close));
78+
79+
IEnumerable<(DateTime d, double v)> baseTuples
80+
= OtherBars.Select(static b => (b.Timestamp, (double)b.Close));
81+
82+
#pragma warning disable CS0618
83+
IReadOnlyList<PrsResult> shim = evalTuples.GetPrs(baseTuples, 20).ToList();
84+
#pragma warning restore CS0618
85+
86+
IReadOnlyList<PrsResult> expected
87+
= ((IReadOnlyList<IReusable>)Bars).ToPrs(OtherBars, 20);
88+
89+
shim.Select(static r => r.Prs).Should().Equal(expected.Select(static r => r.Prs));
90+
}
91+
92+
[TestMethod]
93+
public void GetPrsFromTuplesWithUnspecifiedLookbackComputesWithoutPercent()
94+
{
95+
IEnumerable<(DateTime d, double v)> evalTuples
96+
= Bars.Select(static b => (b.Timestamp, (double)b.Close));
97+
98+
IEnumerable<(DateTime d, double v)> baseTuples
99+
= OtherBars.Select(static b => (b.Timestamp, (double)b.Close));
100+
101+
#pragma warning disable CS0618
102+
IReadOnlyList<PrsResult> shim = evalTuples.GetPrs(baseTuples).ToList();
103+
#pragma warning restore CS0618
104+
105+
IReadOnlyList<PrsResult> expected
106+
= ((IReadOnlyList<IReusable>)Bars).ToPrs(OtherBars);
107+
108+
shim.Should().HaveCount(Bars.Count);
109+
shim.Select(static r => r.Prs).Should().Equal(expected.Select(static r => r.Prs));
110+
shim.Should().OnlyContain(static r => r.PrsPercent == null);
111+
}
112+
113+
[TestMethod]
114+
public void GetPrsMatchesKnownValues()
115+
{
116+
// every other test here compares the shim against ToPrs, so a co-regression in
117+
// ToPrs would move both sides together; these anchor the shim to absolute values
118+
const int lookbackPeriods = 30;
119+
120+
#pragma warning disable CS0618
121+
List<PrsResult> shim = Bars.GetPrs(OtherBars, lookbackPeriods).ToList();
122+
#pragma warning restore CS0618
123+
124+
shim.Should().HaveCount(502);
125+
shim[8].Prs.Should().BeApproximately(0.902250, Money6);
126+
shim[249].Prs.Should().BeApproximately(0.818081, Money6);
127+
shim[249].PrsPercent.Should().BeApproximately(0.023089, Money6);
128+
shim[501].Prs.Should().BeApproximately(0.737019, Money6);
129+
shim[501].PrsPercent.Should().BeApproximately(-0.037082, Money6);
130+
}
131+
132+
[TestMethod]
133+
public void GetPrsRejectsAnExplicitZeroLookback()
134+
{
135+
// v2 rejected <= 0; null was the "no PrsPercent" marker, not 0. This overload
136+
// still distinguishes the two, so an explicit 0 stays an error.
137+
#pragma warning disable CS0618
138+
Action act = () => _ = Bars.GetPrs(OtherBars, 0).ToList();
139+
#pragma warning restore CS0618
140+
141+
act.Should().Throw<ArgumentOutOfRangeException>();
142+
}
143+
144+
[TestMethod]
145+
public void GetPrsRejectsANegativeLookback()
146+
{
147+
#pragma warning disable CS0618
148+
Action act = () => _ = Bars.GetPrs(OtherBars, -5).ToList();
149+
#pragma warning restore CS0618
150+
151+
act.Should().Throw<ArgumentOutOfRangeException>();
152+
}
153+
154+
[TestMethod]
155+
public void GetPrsFromTuplesTreatsZeroAsUnspecified()
156+
{
157+
// pins the one place this shim knowingly diverges from v2, which rejected an
158+
// explicit 0: the 3.0.0 signature narrowed int? to int = 0, leaving 0 as the
159+
// only marker for "unspecified", and restoring the default has to honor it
160+
IEnumerable<(DateTime d, double v)> evalTuples
161+
= Bars.Select(static b => (b.Timestamp, (double)b.Close));
162+
163+
IEnumerable<(DateTime d, double v)> baseTuples
164+
= OtherBars.Select(static b => (b.Timestamp, (double)b.Close));
165+
166+
#pragma warning disable CS0618
167+
List<PrsResult> shim = evalTuples.GetPrs(baseTuples, 0).ToList();
168+
#pragma warning restore CS0618
169+
170+
shim.Should().HaveCount(Bars.Count);
171+
shim.Should().OnlyContain(static r => r.PrsPercent == null);
172+
}
173+
174+
[TestMethod]
175+
public void ErrorLevelGetPrsOverloadAppliesTheSameLookbackMapping()
176+
{
177+
// the four-argument overload is Obsolete(.., true), so C# cannot call it at all
178+
// and CS0619 is not suppressible -- but reflection reaches it, and it carried
179+
// the same defective mapping, so it is verified the only way it can be
180+
MethodInfo overload = typeof(Prs).Assembly
181+
.GetType("FacioQuo.Stock.Indicators.Indicator")
182+
.GetMethods(BindingFlags.Public | BindingFlags.Static)
183+
.Single(m => m.Name == "GetPrs"
184+
&& m.GetParameters().Length == 4
185+
&& m.GetParameters()[0].ParameterType == typeof(IEnumerable<IBar>));
186+
187+
object raw = overload.Invoke(null, [Bars, OtherBars, null, null]);
188+
List<PrsResult> shim = ((IEnumerable<PrsResult>)raw).ToList();
189+
190+
shim.Should().HaveCount(Bars.Count);
191+
shim.Should().OnlyContain(static r => r.PrsPercent == null,
192+
"an unspecified lookback must compute no percent rather than throw");
193+
shim[^1].Prs.Should().BeApproximately(0.737019, Money6);
194+
}
195+
}

0 commit comments

Comments
 (0)