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

Enhancement: Extended Snapshot - Capturing Variables in '__main__' Stack Frame #1004

Merged
merged 14 commits into from
Feb 11, 2024

Conversation

umututku03
Copy link
Contributor

@umututku03 umututku03 commented Jan 26, 2024

Motivation and Context

The current snapshot function gathers variables only from function calls, potentially nested. To enhance its utility, we aim to expand its capabilities, and our goal is to include variables in the top-level scope, specifically those defined in the main block. This expansion caters to a beginner-friendly Python context, providing a more comprehensive snapshot functionality.

Your Changes

Description:

I've updated the snapshot.py module to improve the snapshot function. Now, it captures specific global variables, particularly those defined at the top level within the module where the snapshot function is called. The unit tests have been adjusted accordingly to validate and cover these improvements thoroughly.

Type of change (select all that apply):

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring (internal change to codebase, without changing functionality)
  • Test update (change that modifies or updates tests only)
  • Documentation update (change that modifies or updates documentation only)
  • Other (please specify):

Testing

I've modified the unit tests for the snapshot function and tested the new feature across different levels of nesting. This ensures that the tests now align with the recent changes and thoroughly validate the functionality under various scenarios.

Questions and Comments (if applicable)

N/A

Checklist

  • I have performed a self-review of my own code.
  • I have verified that the CI tests have passed.
  • I have reviewed the test coverage changes reported on Coveralls.
  • I have added tests for my changes.
  • I have updated the CHANGELOG.md file.

@umututku03 umututku03 marked this pull request as ready for review January 26, 2024 04:20
@coveralls
Copy link
Collaborator

coveralls commented Jan 26, 2024

Pull Request Test Coverage Report for Build 7859135880

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 96.904%

Totals Coverage Status
Change from base Build 7803649633: 0.0%
Covered Lines: 3224
Relevant Lines: 3327

💛 - Coveralls

@@ -15,6 +34,9 @@ def snapshot():
frame = inspect.currentframe().f_back

while frame:
if len(local_vars) == 0: # to ensure we do not add main stack frame multiple times
Copy link
Contributor

Choose a reason for hiding this comment

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

This still isn't what I'm looking for. Look at the condition on line 40 and ask why it's there, and how you can use it to address the current problem.

@umututku03
Copy link
Contributor Author

Hi @david-yz-liu! I believe the necessity of the if condition was to only pinpoint local variables, and do not investigate global context. That's why I called the helper function after that check (and with the same indentation level with the if condition) so that the variables within the global context will be added to the accumulator, after the check is made.

@umututku03 umututku03 marked this pull request as draft February 2, 2024 04:51
@umututku03 umututku03 marked this pull request as ready for review February 9, 2024 03:26
@umututku03 umututku03 requested review from david-yz-liu and removed request for david-yz-liu February 9, 2024 03:53
CHANGELOG.md Outdated Show resolved Hide resolved
python_ta/debug/snapshot.py Outdated Show resolved Hide resolved
python_ta/debug/snapshot.py Outdated Show resolved Hide resolved
tests/test_debug/snapshot_main_frame.py Outdated Show resolved Hide resolved
tests/test_debug/snapshot_main_frame.py Outdated Show resolved Hide resolved
tests/test_debug/test_accumulation_table.py Outdated Show resolved Hide resolved
tests/test_debug/test_accumulation_table.py Show resolved Hide resolved
@david-yz-liu david-yz-liu merged commit d077377 into pyta-uoft:master Feb 11, 2024
15 checks passed
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.

3 participants