Closed vi-to closed 5 years ago
This is in the right direction, and appears to work.
When testing, I noticed that the 'rbind' method does not add a 'symbol' column, which makes the data very hard to analyze. Perhaps you could add the column before using do.call to rbind?
Do you mean a 'symbol' to, say, gather the trades regardless of anything else (e.g "GOOG" for each "goog.*" trade, etc.)? I am going to interpret this literally to implement a proof of concept.
At the moment the 'rbind' method returns a data.frame sorted by starting date of the trades, I did this just to introduce a logical criterion. Thus yes, it is likely that the trades will get mixed row-wise (regardless of the symbol), but nonetheless there are ways to subset the data.frame. EDIT(2): Perhaps you mean to remove this characteristic?
To speak my mind, I thought that if one has to run a symbol-based analysis it would be intuitive to input the function only with the symbols of interest or use combn.method='list'
instead to obtain all the statistics symbol by symbol.
The last commit adds what you suggested. Now, as also appear clearer to me, the usage of perTradeStats
for multiple symbols when combn.method='rbind'
is easier.
I named Ticker
the new column for the "parent Symbol" of each trade, both because is meaningful and in order to avoid confusions or tedious problems into the code. Of course, if you have compatibility or usability related concerns with respect to the package, we can easily rename it.
It is called 'Symbol' in the function call, so I wouldn't introduce a new name 'Ticker'.
Also, 'Ticker' is not a precise name in a market. the listed symbol is a precise name. No one has used ticker tape in decades, and tickers were specific to data vendors, not markets.
closes #42
closes #43
Address issue #42 and #43.
This pull request extends
perTradeStats
to accept a vector of symbols included in the portfolio object. In this case, the return type can be decided using the newly introduced parametercombn.method
suggested by @braverock.Please note: the present pull request is intended to replace pull request #79.