braverock / FactorAnalytics

67 stars 63 forks source link

fitSfm function, the Sfm vignette and data used #52

Closed martinrd3D closed 3 years ago

martinrd3D commented 3 years ago

fitSfm and the vignette that Sangeetha wrote are very nice, and it will be great to have them in FA. The fitSfm vignette currently uses the following data sets: StockReturns r.M originally from Berndt (1991) r.W source S+FinMetrics Berndt.dat & folio.dat

TreasuryYields tr.yields text file from companion web site to Ruppert (2010)

We need to replace the above data sets with ones for which we are free to use. For the r.M (monthly) and r.W (weekly) stock data, we can use stocksCRSP. And we can replace tr.yields with identical or similar ones from FRED. This requires revision of the vignette and likely examples used for other functions. I think we should proceed using those data sets to see if there is anything currently broken, and if there is nothing broken, or broken things very easily fixed, and it does cause a build error or warning, then we are in good shape, and only have to replace the data sets, with the attendant revision of the fitSfm vignette and various examples that use the data, and if everything works, it will be justified to make that effort.

kecoli commented 3 years ago

I moved summary.sfm.R to sandbox, given fitSfm has been moved

JustinMShea commented 3 years ago

Thanks @kecoli ! Please make sure you are pulling first and taking time to reconciling any differences before pushing. I've just made a handful of commits so please don't force git push to origin or something similar to overwrite the work. The only file that your commit should impact is summary.sfm.R. Thank you!

martinrd3D commented 3 years ago

But you didn't push right? And I kind of think you are letting Justin do that?

Comment: I see that the other Sfm methods, e.g., print.sfm.R, plot.sfm.R and predict.sfm.R were already put in sandbox 8 days ago by Justin, so what you did completed putting all the Sfm methods in the sandbox, which is good.

On Sun, Aug 29, 2021 at 10:23 PM Ke Li (Kirk) @.***> wrote:

I moved summary.sfm.R to sandbox, given fitSfm has been moved

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/braverock/FactorAnalytics/issues/52#issuecomment-908039112, or unsubscribe https://github.com/notifications/unsubscribe-auth/AP575YYEUQ6FG2B422AYK63T7MIURANCNFSM5CSP3NLA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- R. Douglas Martin Professor Emeritus in Applied Mathematics and Statistics University of Washington

JustinMShea commented 3 years ago

Looks like we can close this issue now? Thanks!