Hi Sina, here’s a general code review. gradboostreg/tree, stat, sample have no tests.
These sub-packages may be better in package gradboostreg since they seem straightforward and contained to this library. Keeping those things in sub-packages doesn’t seem to add much value over just having separate files in my opinion. If you do keep the sub-packages I suggest changing sample.DefaultSample to just sample.Default. I’m not sure I understand why tree.Decider as an interface is necessary. The only use in the package is to return a Decider from NewDecisionTree. I would define such an interface where it’s used. The godoc has no documentation. Do you intend to add an open source license? If not, be sure to read the GitHub default license. Matt On Monday, January 29, 2018 at 4:43:38 PM UTC-6, Sina Siadat wrote: > > Hi Sebastien, > > Thanks for your comment and question :) > > > I have one "drive-by-comment" and a question: > > you could have perhaps used gonum for the stats stuff :) > > Actually, I did start with gonum :)) but I thought it was a large > dependency and I only needed a few funcs from it, so I decided to > copy/write them! > > > and the question: did you compare your package with XGBoost (which is > now kind of a standard candle nowadays) in terms of accuracy, speed and > memory usage? > > I haven't used XGBoost yet, it looks pretty cool, thanks for mentioning > it. I will install it and see if I can compare the two. > > Cheers! > Sina > > -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
