Renaming Total.Net.Profit column to Net.Trading.PL

[scottwcclark]

Although the column is currently named "Total.Net.Profit", the name in the output is "Net.Trading.PL", as the column name is being derived from last(Equity) which has the column name "Net.Trading.PL".
By renaming this column in the ret and tmpret data frames the column names in the output won't change when tradeStats is being called on a portfolio with transactions, however, the output will be different when tradeStats is run on a portfolio without transactions as the column name will change from "Total.Net.Profit" to "Net.Trading.PL". By renaming this it will prevent the matching error which occurs when trying to rbind the tradeStats of the portfolios with transactions ("Net.Trading.PL") with the portfolios without ("Total.Net.Profit") when using the apply.paramset function.

"Net.Trading.PL" and "Total.Net.Profit" are not technically the same but since the former is currently being returned in the output anyway would it be safe to say that we could rename the latter with not a great deal of change?

[jaymon0703]

Thanks @scottwcclark i will test it later

[scottwcclark]

Hi @jaymon0703 just wanted to check in with this one, did you have any luck testing it? If not, is it still the case that a solution will probably need to be incorporated into apply.paramsets instead?

[jaymon0703]

Hi @scottwcclark thanks for checking in. This fell off my radar. Looking at the conversation in the issue created for this, braverock/quantstrat#121, i dont think changing the name is an ideal solution. Instead, we should handle a case where a certain param.combo returns no trades, and ideally make this visible to the user. I should probably focus on that next. There is potentially some more work to do in apply.paramset for a new feature, parallelizing applyStrategy (EDIT: Link to new issue for this work in quantstrat - braverock/quantstrat#124). It might be a good opportunity to do both pieces of work, although unrelated they will impact the same function.

Is the issue of apply.paramset not gracefully handling zero-trade strategies a recurring problem for you?

[jaymon0703]

Hi @scottwcclark please test feature branch 121_apply.paramset_handle_no_trades? This should be resolved with
braverock/quantstrat@ddf64fe.

To install that particular branch, you can use install_github("braverock/quantstrat", ref = "121_apply.paramset_handle_no_trades").

[scottwcclark]

Hi @jaymon0703 this was something that I encountered quite a lot, although its probably not very common for most users.

I have just installed the branch that you mentioned and checked with the reproducible example I used when creating this issue and I'm glad to say it worked just as I hoped!

Thank you very much for your help with this, the "results" environment contained the results for both of the param combos and was not null so that's great, I'll now be able to see the TradeStats for all of the successful combination.

Thanks again!

Scott

[jaymon0703]

Great thanks for confirming @scottwcclark. Closing this PR and will merge the feature branch to master in quantstrat, with a corresponding minor version bump.