-
Notifications
You must be signed in to change notification settings - Fork 133
Use co_qualname in reporting when possible #345
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #345 +/- ##
==========================================
+ Coverage 64.84% 64.96% +0.12%
==========================================
Files 13 13
Lines 1041 1056 +15
Branches 228 233 +5
==========================================
+ Hits 675 686 +11
- Misses 306 309 +3
- Partials 60 61 +1 Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
TTsangSC
left a comment
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.
Hi, thanks for having me over for the review.
As previously discussed this is highly useful for end-users, and honestly I'm surprised how little of the test suite we broke (we have a ton of assert 'Function: <something>' in raw_output), but apparently none of that has to do with methods in classes so we're in the clear. I'd expect breakages in downstream tools to be similarly contained, manageable, and worth the change.
Maybe we can tweak the docstring for line_profiler._line_profiler.label() a bit (your call), but otherwise we've good to go methinks.
|
The breakage is because the output is now |
line_profiler/_line_profiler.pyx
Outdated
| NOP_BYTES: bytes = NOP_VALUE.to_bytes(2, byteorder=byteorder) | ||
|
|
||
| # This should be true for Python >=3.11a1 | ||
| HAS_CO_QUALNAME: bool = hasattr(types.CodeType, 'co_qualname')) |
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.
))
Oops
|
Nice, should be good to go once the CHANGELOG is fixed |
Report with co-qualname as discussed in #337 (comment)
I'm simply using a check for AttributeError to try to see if qualname is available, and fallback to name otherwise. I could check for the exact python version, but this seems fine. A few tests had to change to account for both name and qualname behaviors happening.