Skip to content
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.

Sign up for GitHub

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

Draft: Add a decorator method to record function calls with parameters #97

Closed
wants to merge 6 commits into from
Closed

Draft: Add a decorator method to record function calls with parameters #97

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f41ef16
Add a decorator method to record function calls with parameters
zain-sohail Jan 30, 2023
6b57ead
add class to call tracking
rettigl Feb 1, 2023
2c87a50
Keep state of all functions that hold this decorator
zain-sohail Feb 1, 2023
914975a
linting
rettigl Feb 1, 2023
8ca91e0
merge
zain-sohail Feb 1, 2023
356c02e
Fix behaviour fr not all arguments are passed
zain-sohail Feb 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sed/core/workflow_recorder.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,6 @@ def __init__(self, func):

def __call__(self, *args, **kwargs):
self.__class__.call_tracker.append(
MethodCall(self.func.__name__, args, kwargs),
MethodCall(self.func.__qualname__, args, kwargs),
)
return self.func(*args, **kwargs)
return self.func(self, *args, **kwargs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test that? I understand that this will pass the MethodCall instance instead of the one of the calling function.

At least when trying to instantiate the sed processor now, I get:

AttributeError: 'CallTracker' object has no attribute 'loader'

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I did test that, with the same example I provided above.

You are not wrong but the update_wrapper argument basically makes the MethodCall instance into a Foo instance, from what I understood. I used it in the functional wrapper approach as well (but it uses a different function there).

I will try to get it to work with loader and other methods we have.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daviddoji if you have a better understanding of decorators, would be very helpful :)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Never played too much with decorators (just silly ones than do timing). Maybe have a look at some examples in here: https://wiki.python.org/moin/PythonDecoratorLibrary

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also think that you should return self.func(*args,**kwargs) as the decorator should then just operate the function as normal.
It seems to me that adding self adds an argument to the function before calling...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately that does not pass the self argument to the method, in the case where you don't provide all the arguments (default ones). This one worked for me for a test case but apparently isn't for the real situation. So definitely buggy and I need to understand better the handling.