Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Fix type annotation ofpstats.FunctionProfile.ncalls#96741

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

@ruancomelli
Copy link
Contributor

This PR aligns the type annotation ofpstats.FunctionProfile.ncalls with its runtimestr type.

The fact that the runtime type ofpstats.FunctionProfile.ncalls isstr can be verified by (1) inspecting the source code forpstats and (2) running a small test snippet:

  1. in the source code forpstats, the only place wherencalls is defined seems to be

    ncalls=str(nc)ifnc==ccelse (str(nc)+'/'+str(cc))
    where the assigned value is always a string;

  2. you can also check thatstats.FunctionProfile.ncalls is a string by running the following script:

    fromcProfileimportProfilefrompstatsimportStats# execute random code to generate profile statswithProfile()asprofile:print("foo")# get a sample function profilestats_profile=Stats(profile).get_stats_profile()function_profile=next(iter(stats_profile.func_profiles.values()))# check that the type of `function_profile.ncalls` is indeed `str`asserttype(function_profile.ncalls)isstr

I also have an open PR intypeshed to fix this issue there:python/typeshed#8712.

This change seems small and straightforward enough not to require an issue, but I am happy to open one if you think it is necessary. I also did not include any tests since this is a type annotation fix without any changes to the runtime behavior.

This change aligns the type annotation of `pstats.FunctionProfile.ncalls` with its runtime type.
@bedevere-bot
Copy link

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

@ghost
Copy link

ghost commentedSep 10, 2022•
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-bot
Copy link

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

@bedevere-bot
Copy link

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

Copy link
Member

@AlexWaygoodAlexWaygood left a comment

Choose a reason for hiding this comment

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

LGTM!

This change seems small and straightforward enough not to require an issue, but I am happy to open one if you think it is necessary. I also did not include any tests since this is a type annotation fix without any changes to the runtime behavior.

I agree that this probably doesn't need an issue or NEWS entry, since there's no user-visible change introduced by this PR. (Type checkers use information from typeshed for the stdlib, rather than looking at any annotations in the CPython codebase.) I think the change is still valuable, however, since it's highly confusing forreaders of the code for there to be an incorrect annotation here.

I don't really have an opinion on whether this should be backported or not -- on the one hand, there's no user-visible change here; on the other hand, there's no real risk to it being backported.

@AlexWaygood
Copy link
Member

@gpshead, as the person who reviewed and merged#15495, which introduced this class -- does this look okay to you?

@gpshead
Copy link
Member

It's still useful to add a NEWS entry saying "corrected the type annotation on data class blah.blah.thing to be str."

AlexWaygood and ruancomelli reacted with thumbs up emoji

@gpsheadgpshead added needs backport to 3.10only security fixes needs backport to 3.11only security fixes and removed skip news labelsSep 14, 2022
@bedevere-bot
Copy link

Most changes to Pythonrequire a NEWS entry.

Please add it using theblurb_it web app or theblurb command-line tool.

@ruancomelli
Copy link
ContributorAuthor

Hi@gpshead, thanks for the suggestion. I added a NEWS file usingblurb_it.
Since there is no GitHub issue linked to this PR, and IDs are unique for PRs and issues, I used this PR's ID96741 when generating the NEWS file. Please let me know if this somehow breaks the usual merging workflow.

@gpsheadgpshead merged commit8e9a37d intopython:mainSep 15, 2022
@miss-islington
Copy link
Contributor

Thanks@ruancomelli for the PR, and@gpshead for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

@bedevere-bot
Copy link

GH-96835 is a backport of this pull request to the3.11 branch.

@bedevere-botbedevere-bot removed needs backport to 3.11only security fixes needs backport to 3.10only security fixes labelsSep 15, 2022
@bedevere-bot
Copy link

GH-96836 is a backport of this pull request to the3.10 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestSep 15, 2022
* fix: annotate `pstats.FunctionProfile.ncalls` as `str`This change aligns the type annotation of `pstats.FunctionProfile.ncalls` with its runtime type.(cherry picked from commit8e9a37d)Co-authored-by: Ruan Comelli <ruancomelli@gmail.com>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull requestSep 15, 2022
* fix: annotate `pstats.FunctionProfile.ncalls` as `str`This change aligns the type annotation of `pstats.FunctionProfile.ncalls` with its runtime type.(cherry picked from commit8e9a37d)Co-authored-by: Ruan Comelli <ruancomelli@gmail.com>
AlexWaygood added a commit to python/typeshed that referenced this pull requestSep 15, 2022
The annotation in CPython was fixed thanks to@ruancomelli inpython/cpython#96741!
@ruancomelliruancomelli deleted the fix-pstats-functionprofile-ncalls-annotation branchSeptember 15, 2022 10:20
AlexWaygood added a commit to python/typeshed that referenced this pull requestSep 15, 2022
The annotation in CPython was fixed thanks to@ruancomelli inpython/cpython#96741!
ambv pushed a commit that referenced this pull requestOct 5, 2022
…) (#96835)This change aligns the type annotation of `pstats.FunctionProfile.ncalls` with its runtime type.(cherry picked from commit8e9a37d)Co-authored-by: Ruan Comelli <ruancomelli@gmail.com>
ambv pushed a commit that referenced this pull requestOct 5, 2022
…) (#96836)This change aligns the type annotation of `pstats.FunctionProfile.ncalls` with its runtime type.(cherry picked from commit8e9a37d)Co-authored-by: Ruan Comelli <ruancomelli@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@gpsheadgpsheadgpshead approved these changes

@JelleZijlstraJelleZijlstraJelleZijlstra approved these changes

@AlexWaygoodAlexWaygoodAlexWaygood approved these changes

Assignees

No one assigned

Labels

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ruancomelli@bedevere-bot@AlexWaygood@gpshead@miss-islington@JelleZijlstra
[8]ページ先頭
©2009-2025 Movatter.jp