Skip to content

BUG: Return 0 from Trade.pl_pct when entry_price is 0 or NaN - #1416

Open
huixin3 wants to merge 1 commit into
kernc:masterfrom
huixin3:fix-trade-pl-pct-zero-division
Open

huixin3 wants to merge 1 commit into
kernc:masterfrom
huixin3:fix-trade-pl-pct-zero-division

Conversation

@huixin3

@huixin3 huixin3 commented Oct 2, 2026

Copy link
Copy Markdown

Fixes #1318.

Trade.pl_pct divided by entry_price unconditionally, raising
ZeroDivisionError when it was 0 and returning NaN when it was NaN.
It now returns 0 in both cases.

Added a regression test test_trade_pl_pct_zero_entry_price
covering both inputs; verified it fails without the fix.

@arhancanli

Copy link
Copy Markdown

The guard fixes the ZeroDivisionError, but returning 0. changes what the stats see. _stats.py:71 builds ReturnPct from t.pl_pct, and _stats.py:91 takes returns = trades_df['ReturnPct'] for the trade-level numbers (win rate, average and best/worst trade, expectancy, SQN). A NaN pl_pct was skipped by pandas' mean/std, so a bad-entry trade dropped out of those; a 0. is counted as a real break-even trade, which pulls win rate down and shrinks the mean and SQN, and nothing in the output says a trade had an unusable entry price.

So for the NaN case the new behaviour is arguably worse than before: it turns "no information" into "a trade that returned exactly 0%". Returning float('nan') for both inputs (and keeping the guard so zero no longer raises) would keep the stats honest, and the test could assert np.isnan(trade.pl_pct) plus that stats['# Trades'] still counts it while Win Rate [%] is computed over the non-NaN trades. If you do want 0., a docstring line saying such trades count as break-even would at least make it a documented choice.

Smaller point: Trade.pl (line 663) still uses entry_price without a guard, so with NaN it returns NaN while pl_pct now says 0; the two disagree for the same trade.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CGPT5 proposed enhancements

2 participants