Added logger decorator to all functions #841
Conversation
|
@Daniel-olaO I prefer the attribute approach. Adding decorators to every function, although is more explicit, but is less maintainable. |
|
(especially since it's just for logging, if it were for something more substantial like just-in-time compilation, adding the decorator to function header would make sense since developers should really know what's going on, whereas for logging, the passive approach seems more sensible to me), but let's wait for @jgostick to weigh in before you make the changes to not waste your time. Also, I really appreciate your efforts :) |
|
okay thanks @ma-sadeghi |
|
There are pros/cons for both approaches. The decorator is nice because it let's us explicitly pick and choose which functions to emit the logger message from. I think @Daniel-olaO has gone a bit overboard by putting them on ALL the functions. If we just do it on the really heavy functions, then I think the maintenence would not be too bad, AND the approach would be explicit. On the otherhand, @ma-sadeghi's module attribute approach only requires code in ONE place, which is really tight. The downside of this approach to me is that it also represents a maintenence issue in the future when we have to ask ourselves WTF these logger messages are coming from! |
|
hI @jgostick, why not write a list of functions you want me to add the logger decorator |
|
Another perspective: do we even need this? Profiling should be done by the user, and there are many great tools that exactly does this for us: scalene, memprofiler, etc. They provide per function runtime, memory usage, and even nice flame graphs. |
close #838