-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Added log message to the begining and ending of every functions #839
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Daniel-olaO Thanks for the PR. Quick question: why do you use inspect
to get function name? That's 2 extra lines of code per function (3 instead of 1), why not just hardcode the function name?
Also, please capitalize "start" and "end" for consistency. I think you also need to add a space between "start of" and function name.
Hi @ma-sadeghi, I suggested that the inspect module could be used to get the function name. That way if we every change the function name or just cut-paste the code it will be self-healing. |
Hey @jgostick, I see. What do you think about wrapping the "start" and "end" snippets in two private helpers, and just call them at the start/end of function calls, like import inspect
def caller_function():
# Get the frame of the caller function
frame = inspect.currentframe().f_back
# Get the name of the caller function
caller_name = frame.f_code.co_name
return caller_name
def test_function():
caller = caller_function()
print("Caller function:", caller)
test_function() This would get rid of ~300-400 extra lines. |
@Daniel-olaO I appreciate how thorough you were! I didn't realize you were going to be so quick to implement this, and figured we'd have time to discuss and plan a bit. I think that we should be a bit more judicous with this feature an only put it in the "front-line" functions, and especially those that have the potential to be slow. For instance, anything that starts with an underscore is a hidden function that is called by something else and anything that is Another thought...maybe we could jam all the inspect code into a single line? That would make it harder to read, but easier to maintain. Then we can cut-paste the code to new functions as needed for instance. |
Not to make this overly complicated, but I wonder if this could be done as a decoarator? I know we could do the 'start' but not sure about the 'end'. Like: @functimer
def function_name(args, kwargs):
<code here>
def functimer():
<code here> I have not really dove into decorators so don't know the ins-and-outs, but this seems like a possibility. |
Hey @jgostick, thank you for your input. I am considering implementing the solution suggested by @ma-sadeghi as it appears more apparent to me. What do you think about this? |
@jgostick @Daniel-olaO I found an elegant workaround; the good thing about it is that it leaves all of our functions untouched, and requires a small modification to our modules' Basically, what we need to do is to write a decorator (as @jgostick suggested) to be added to function entry/exit, and then apply it automatically to all our top-level functions as attribute. I've tested it on import sys
import logging
import functools
logger = logging.getLogger(__name__)
def log_entry_exit(func):
@functools.wraps(func)
def wrapper(*args, **kwargs):
logger.info(f"Entering {func.__name__}")
result = func(*args, **kwargs)
logger.info(f"Exiting {func.__name__}")
return result
return wrapper
current_module = sys.modules[__name__]
for func_name, func in vars(current_module).copy().items():
if callable(func):
setattr(current_module, func_name, log_entry_exit(func)) @jgostick What do you think? @Daniel-olaO In case we want to move forward, move |
@ma-sadeghi, wow that is slick. Will it apply to EVERY function? That could be a problem for functions that are called repeatedly. Do you think this will be a problem, and if so is there any way to specify which functions it can be applied to? |
Good point. Currently, it'll apply to every accessible "top level" function within that module, both with or without underscore. I tried it on add_boundary_regions
generate_voxel_image
label_phases
label_boundaries
map_to_regions
maximal_ball_wrapper
regions_to_network
snow2
_net_dict
_parse_pad_width
diffusive_size_factor_AI
create_model
find_conns We could add a check like If we want to have more control, we can add a list of functions we want to profile to |
Hey @ma-sadeghi, and @jgostick I have created the |
Great. You mean adding the decorator to each function? (before function header) |
oh thanks, work on it |
Hello everyone, I apologize for creating another PR with the number #839. I had to do this to resolve the logger issue. The reason for this is because I created a new branch from the dev branch to eliminate all the changes I made. |
close #838