search

joshuaulrich / TTR

Technical analysis and other functions to construct technical trading rules with R
GNU General Public License v2.0
330 stars 103 forks source link

Extend the Close-to-Close volatility estimator #9

Closed jtoll closed 9 years ago

jtoll commented 9 years ago

so that there's now the option to use a mean of 0 when calculating the SD by selecting the "close0" method.

joshuaulrich commented 9 years ago

A few changes, and it will be ready to go: 1) Use isTRUE(mean0) instead of mean0==TRUE 2) Please fixup/squash into a single commit. 3) Add the rationale for the change and issue number to the commit message. For example:

Allow Cl/Cl volatility to assume mean=0

The sample mean can be biased in the case of small n with a strongly trending underlying, resulting in an underestimation of volatility.

Add mean0 argument that will assume a sample mean of zero when TRUE. See #8.

jtoll commented 9 years ago

I'm happy to make all the changes you've suggested. I think the only real loss of code from squashing into one commit will be the reverted documentation section for the close0 method.

joshuaulrich commented 9 years ago

Sorry, I don't follow. The point of squashing is to remove the calc="close0" changes, so I'm not merging reverted commits.

Here's what I would do: add a commit to make the isTRUE change (and you could re-add the vClose0 example while you're at it). Then rebase -i master and squash the 2nd and 3rd commits into the first and edit the commit message. Then force push.

jtoll commented 9 years ago

Sorry, I think I was a unclear, but I think we were thinking the same thing. Anyway, I think this is right. Please let me know otherwise. Thanks.

joshuaulrich commented 9 years ago

Thanks for the contribution!