cdipaolo/goml

Remove `fmt.Printf`s?

mitchellh opened this issue · 5 comments

Hello!

Great library. I noticed during tests that the code decides to just fmt.Printf. I don't want the ML lib in my app to be outputting to the console without me knowing. Can we disable that? Or provide a way to provide an alternate io.Writer?

Thanks!

This sounds like a good idea. I'm pretty stacked with work today but should be able to prototype something with the io.Writer tomorrow I think. I'll let you know.

I'm likely going to add an io.Writer field to the model structs and just change the fmt.Printf calls to fmt.Fprintf, which (as seen below from the fmt package code) is equivalent if you set your writer to os.Stdout, so I'll default to that so there are no breaking changes. If you have any comments let me know.

I wouldn't want to remove the printing completely in case someone was relying on that feature for anything in an application. Similarly I think that the output is potentially nice if you're debugging something.

from https://golang.org/src/fmt/print.go

   184  // Fprintf formats according to a format specifier and writes to w.
   185  // It returns the number of bytes written and any write error encountered.
   186  func Fprintf(w io.Writer, format string, a ...interface{}) (n int, err error) {
   187      p := newPrinter()
   188      p.doPrintf(format, a)
   189      n, err = w.Write(p.buf)
   190      p.free()
   191      return
   192  }
   193  
   194  // Printf formats according to a format specifier and writes to standard output.
   195  // It returns the number of bytes written and any write error encountered.
   196  func Printf(format string, a ...interface{}) (n int, err error) {
   197      return Fprintf(os.Stdout, format, a...)
   198  }

That sounds perfect. Thanks!

Tests Before Changes

➜  goml git:(master) go test ./...
ok      github.com/cdipaolo/goml/base   0.252s
ok      github.com/cdipaolo/goml/cluster    14.893s
ok      github.com/cdipaolo/goml/linear 46.546s
ok      github.com/cdipaolo/goml/perceptron 9.365s
ok      github.com/cdipaolo/goml/text   0.009s

Tests After Changes

➜  goml git:(logging) go test ./...
ok      github.com/cdipaolo/goml/base   0.287s
ok      github.com/cdipaolo/goml/cluster    11.707s
ok      github.com/cdipaolo/goml/linear 38.457s
ok      github.com/cdipaolo/goml/perceptron 9.029s
ok      github.com/cdipaolo/goml/text   0.006s

The speedup is a little surprising because I basically just removed one or two function calls per test. I suspect that printing to dev/null (with ioutil.Discard) might speed up tests a lot.

All models that did print to log not have an

        // Output is the io.Writer used for logging
        // and printing. Defaults to os.Stdout.
        Output io.Writer

struct field as a public variable that can be changed by the user. All calls to fmt.Printf were switched to fmt.Fprintf.

If you have any other comments let me know and I can reopen this.