cmu-delphi / epipredict

Tools for building predictive models in epidemiology.
https://cmu-delphi.github.io/epipredict/
Other
8 stars 8 forks source link

feat: canned forecasters print their args by default #310

Closed dshemetov closed 2 months ago

dshemetov commented 3 months ago

Checklist

Please:

Change explanations for reviewer

Adds .verbose = TRUE to canned forecasters and prints their arguments if TRUE.

Magic GitHub syntax to mark associated Issue(s) as resolved when this is merged into the default branch

dshemetov commented 3 months ago
  1. I think I leaned to the noisy default because I figured this is most useful for a beginner user of this package and I didn't want them to have to hunt for this flag. The more experienced users can turn it off and write a default-off wrapper for themselves if the extra typing is really bothering them.
  2. My thinking was that both functions are user-facing, the workflow creator is just for a more advanced user. If that's true, I could see defaulting to off in the workflow creator and on in the simpler one.
dajmcdon commented 2 months ago

In R, I don't think it's customary to print things by default (with the possible exception of a progress bar, though even that is usually silent). I'd prefer these to be off.

Come to think of it, I think we should add these to the print methods for the resulting object rather than altering the function signature at all.

dshemetov commented 2 months ago

I hear ya about not wanting to be too noisy.

I like the idea of adding this to the object printer. Let me see what I can do there.

dshemetov commented 2 months ago

After thinking about it some more, doesn't seem worth it.