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.

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

Wrap all StdHook methods #6

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Wrap all StdHook methods #6

wants to merge 2 commits into from

Conversation

Wauplin
Copy link

@Wauplin Wauplin commented Jul 20, 2021

Hi !

First of all, thanks a lot for this package and for the Deepkit software in general ! We are using it to share experiments internally in our team and it helps a lot 😃

I encountered an issue when trying to debug my code using pdb. It has to do with the terminal (stdout) that is catched around the StdHook wrapper that logs messages to Deepkit. When I use pdb, I cannot use the arrows to retrieve previous expressions. It is the same issue as described in this issue (even though it's a different package and the source of the problem is different).

I have made a workaround to redirect all calls to StdHook object to the inner object self.s, except for the write method which is the one that needs to be overloaded. Maybe this slight improvement can be useful for other people 🙂

@marcj
Copy link
Member

marcj commented Jul 20, 2021

Thanks @Wauplin! :) Which python version do you use? I added comments to the changes itself, as I'm not sure about the fix itself yet.

def write(self, s):
self.s.write(s)
log(s)

def __getattr__(self, name):
if name == "s":
super(StdHook, self).__getattr__(name)
Copy link
Member

@marcj marcj Jul 20, 2021

Choose a reason for hiding this comment

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

shouldn't this have a return statement? Or what is this supposed to do? super(StdHook) doesn't look right to me since StdHook has no parent class? I'm not sure I understand this fix.

return super(StdHook, self).__getattr__(name)

@Wauplin
Copy link
Author

Wauplin commented Jul 21, 2021

Hi @marcj, thanks for the quick feedback

Which python version do you use?

I'm using Python 3.8.10 and I have to admit, I didn't try any other version.

shouldn't this have a return statement?

Yes, my mistake.

Or what is this supposed to do? super(StdHook) doesn't look right to me since StdHook has no parent class?

I have changed a bit the fix and added some comments with some references to explain basically what's happening. In fact, super(StdHook) is just returning the base class object which implements the base __setattr__.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants