Skip to content

Fix parameter names in Backtest.optimize example - #1414

Closed
jiajunchang2002g wants to merge 3 commits into
kernc:masterfrom
jiajunchang2002g:master
Closed

jiajunchang2002g wants to merge 3 commits into
kernc:masterfrom
jiajunchang2002g:master

Conversation

@jiajunchang2002g

Copy link
Copy Markdown

Fixes #1413

@arhancanli

Copy link
Copy Markdown

This PR currently shows 0 changed files: the branch is jiajunchang2002g:master, and its last commit (0df0481) is a revert of the merge that carried the docstring edit, so it contains nothing to review or merge. The fix for #1413 would need to be pushed again (ideally from a topic branch, not master).

For whoever re-pushes it, the line in question is the optimize() docstring at backtesting/backtesting.py ~L1452-1453 on master:

best_stats = backtest.optimize(sma1=[5, 10, 15], sma2=[10, 20, 40],
                               constraint=lambda p: p.sma1 < p.sma2)

Renaming to n1/n2 (the class variables in the Quick Start SmaCross) keeps the sentence above it correct: with n1 < n2, the pairs left of the 9 are (5,10), (5,20), (5,40), (10,20), (10,40), (15,20), (15,40), which is the "7 admissible (of the 9 possible)" the text claims. Note that the constraint lambda has to be renamed too (p.n1 < p.n2), otherwise the example fails with the AttributeError from the issue, just on a different line. Another option is a one-line sentence saying the names must match the strategy's class variables, since the docstring example does not name a strategy.

I read the file through the GitHub API and counted the pairs by hand; I did not run the example.

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.

Backtesting.backtesting optimize() documentation example kwarg variables should be n1 and n2 instead of sma1 and sma2

2 participants