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

Feature request: Support depth=float('inf') #200

Open
cool-RR opened this issue Oct 15, 2020 · 8 comments
Open

Feature request: Support depth=float('inf') #200

cool-RR opened this issue Oct 15, 2020 · 8 comments

Comments

@cool-RR
Copy link
Owner

cool-RR commented Oct 15, 2020

No description provided.

@elijahqi
Copy link

Claim, I will try to add this feature

@cool-RR
Copy link
Owner Author

cool-RR commented Mar 29, 2023

Great. It's likely easy.

@elijahqi
Copy link

Hi!I wanted to discuss a few ideas with you to ensure that my changes align with the project's goals.

I've identified a line in trace.py (line 406) that might need modification. Specifically, there's a for loop that iterates from 0 to depth. I propose introducing an if statement to create two branches:

If self.depth == inf, the loop would be modified to continue up the call stack until there are no more frames to trace.
Otherwise, the original code would be retained.
Additionally, I'd like to inquire about any quality assurance strategies or guidelines you'd like me to follow. I noticed that the project utilizes pytest for unit testing, which I think is essential for this change. If you have any other recommendations or specific test cases you'd like me to consider, please let me know.

Thank you for your time, and I look forward to your feedback.

@cool-RR
Copy link
Owner Author

cool-RR commented Apr 12, 2023

Hi!I wanted to discuss a few ideas with you to ensure that my changes align with the project's goals.

I've identified a line in trace.py (line 406) that might need modification. Specifically, there's a for loop that iterates from 0 to depth. I propose introducing an if statement to create two branches:

If self.depth == inf, the loop would be modified to continue up the call stack until there are no more frames to trace. Otherwise, the original code would be retained.

Kind of, yes, but do it like this:

depth_iterator = itertools.count(1) if (self.depth == float('inf')) else range(1, self.depth)
for i in depth_iterator:

Additionally, I'd like to inquire about any quality assurance strategies or guidelines you'd like me to follow. I noticed that the project utilizes pytest for unit testing, which I think is essential for this change. If you have any other recommendations or specific test cases you'd like me to consider, please let me know.

Thank you for your time, and I look forward to your feedback.

Write tests for it without my guidance and show me.

@elijahqi
Copy link

ok. Thank you!

@elijahqi
Copy link

elijahqi commented Apr 16, 2023

Hi!I wanted to discuss a few ideas with you to ensure that my changes align with the project's goals.
I've identified a line in trace.py (line 406) that might need modification. Specifically, there's a for loop that iterates from 0 to depth. I propose introducing an if statement to create two branches:
If self.depth == inf, the loop would be modified to continue up the call stack until there are no more frames to trace. Otherwise, the original code would be retained.

Kind of, yes, but do it like this:

depth_iterator = itertools.count(1) if (self.depth == float('inf')) else range(1, self.depth)
for i in depth_iterator:

Additionally, I'd like to inquire about any quality assurance strategies or guidelines you'd like me to follow. I noticed that the project utilizes pytest for unit testing, which I think is essential for this change. If you have any other recommendations or specific test cases you'd like me to consider, please let me know.
Thank you for your time, and I look forward to your feedback.

Write tests for it without my guidance and show me.

I also observed an issue. After implementing this feature, the program can trace an infinite depth, which results in the display of system functions, such as:

def currentframe(): 20:41:59.533845 line 1751 return sys._getframe(1) if hasattr(sys, "_getframe") else None 20:41:59.533854 return 1751 return sys._getframe(1) if hasattr(sys, "_getframe") else None

This scenario can also occur in the original code when setting the depth to a significantly large value.

For instance, consider the following test case:

import pysnooper

@pysnooper.snoop(depth = 999)
def func(x):
if x == 0:
return 1
return x * func(x - 1)

func(5)

I am uncertain whether we should halt tracing when such situations arise.

@cool-RR
Copy link
Owner Author

cool-RR commented Apr 16, 2023

That is a potential problem, but it's unrelated to this change. So go forward with this change, and after it's merged, if you'll be interested you can investigate this problem. One good first step in investigating this problem is see whether it happens with https://github.com/alexmojaki/snoop .

Also, when you copy-paste code to GitHub, make sure you're using a multiline code block rather than single line, and make sure it's properly formatted. (Both code samples in your last message don't show properly.)

@elijahqi
Copy link

Thank you! Indeed, it appears that the issue isn't directly tied to this particular change. I've had the opportunity to review the project you provided, and it doesn't exhibit the same problem. While the modification to float('inf') isn't the root cause of the issue, it did create complications when I was writing tests. To address this, I attempted to resolve the aforementioned problem. The issue seems to stem from the code not verifying if the current frame is an internal frame. To tackle this, I introduced a condition that checks if the frame candidate is an internal frame; if so, it returns None, indicating that it will not be traced.

if not (frame.f_code in self.target_codes or frame in self.target_frames):
            if self.depth == 1:
                # We did the most common and quickest check above, because the
                # trace function runs so incredibly often, therefore it's
                # crucial to hyper-optimize it for the common case.
                return None
            elif self._is_internal_frame(frame):
                return None
            else:
                _frame_candidate = frame
                depth_iterator = itertools.count(1) if (self.depth == float('inf')) else range(1, self.depth)
                for i in depth_iterator:
                    _frame_candidate = _frame_candidate.f_back
                    if _frame_candidate is None:
                        return None
                    elif self._is_internal_frame(_frame_candidate):
                        return None
                    elif _frame_candidate.f_code in self.target_codes or _frame_candidate in self.target_frames:
                        break
                else:
                    return None

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

No branches or pull requests

2 participants