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

gh-127610: Added validation for more than one var positional and var keyword parameters in inspect.Signature#127657

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

Merged
serhiy-storchaka merged 10 commits intopython:mainfromApostolFet:fix-issue-127610
Dec 8, 2024

Conversation

@ApostolFet
Copy link
Contributor

@ApostolFetApostolFet commentedDec 5, 2024•
edited
Loading

Added flags for var positional and var keyword parameters.
Raise ValueError if arguments are more than one.
Added corresponding tests

@ghost
Copy link

ghost commentedDec 5, 2024•
edited by ghost
Loading

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app
Copy link

Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply theskip news label instead.

Copy link
Contributor

@skirpichevskirpichev left a comment

Choose a reason for hiding this comment

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

And please add news. This is tiny, but user-visible change.

Comment on lines 2977 to 2978
var_positional_count+=1
ifvar_positional_count>1:
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems,*_count actually didn't count anything, isn't? I suggest use bool values instead and rename variables to something likeseen_var_positional.

ifkind==_VAR_POSITIONAL:
var_positional_count+=1
ifvar_positional_count>1:
msg='more than one var positional parameter'
Copy link
Contributor

Choose a reason for hiding this comment

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

Perglossary, probably it is better to use "var-positional" (and "var-keyword") terms.

@bedevere-app
Copy link

Most changes to Pythonrequire a NEWS entry. Add one using theblurb_it web app or theblurb command-line tool.

If this change has little impact on Python users, wait for a maintainer to apply theskip news label instead.

Copy link
Contributor

@skirpichevskirpichev left a comment

Choose a reason for hiding this comment

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

LGTM, with a small nitpick.

CC@serhiy-storchaka

…tv_NP.rstCo-authored-by: Sergey B Kirpichev <skirpichev@gmail.com>
Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

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

You do not actually needseen_var_positional andseen_var_keyword.seen_var_positional is equal totopkind >= _VAR_POSITIONAL (before settingtopkind = kind).

You can addif kind == top_kind and kind in (_VAR_POSITIONAL, _VAR_KEYWORD) beforeelif kind > top_kind. Usekind.description to format the error message.

Comment on lines 2995 to 2997
second_args=args.replace(name="second_args")
withself.assertRaisesRegex(ValueError,'more than one var-positional parameter'):
S((args,second_args))

Choose a reason for hiding this comment

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

Add also a test for the case when there are other parameters (keyword-only or var-keyword) between two var-positional parameters.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

In this case we will get error:ValueError: wrong parameter order: keyword-only parameter before variadic positional parameter. Should we catch it in the test?

My test:S((args, ko, second_args))

Choose a reason for hiding this comment

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

Technically, there are two errors: "keyword-only parameter before variadic positional parameter" and "more than one variadic positional parameter". Which of them are preferable to report? I think the latter, therefore we should change the order of checks. And tests are needed to ensure that the correct error message is used.

Copy link
ContributorAuthor

@ApostolFetApostolFetDec 7, 2024•
edited
Loading

Choose a reason for hiding this comment

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

In that case we all need the variables seen_var_positional and seen_var_keyword, to check that these parameters have not occurred before even if the parameter order is wrong, right?
if kind == top_kind and kind in (_VAR_POSITIONAL, _VAR_KEYWORD) - this check is designed for the correct order of parameters

Choose a reason for hiding this comment

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

Hmm, yes, you are right. So you need to simply move the new checks up. I still suggest to usekind.description to format the error message.

You can unify the code for var-positional and var-keyword if use a set instead of two boolean variables.

ifkindin (_VAR_POSITIONAL,_VAR_KEYWORD):ifkindinseen_var_parameters:raise ...seen_var_parameters.add(kind)

Copy link
Member

@serhiy-storchakaserhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM. 👍

You can use f-string for formatting the error message.

Copy link
Member

@picnixzpicnixz left a comment

Choose a reason for hiding this comment

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

I'm ok with this current form and you can change to f-strings as Serhiy said if you want (but forget about myset vs tuple suggestion)

@serhiy-storchakaserhiy-storchaka merged commit1503fc8 intopython:mainDec 8, 2024
38 checks passed
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotaarch64 Android 3.x has failed when building commit1503fc8.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1594/builds/759) and take a look at the build logs.
  4. Check if the failure is related to this commit (1503fc8) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1594/builds/759

Summary of the results of the build (if available):

Click to see traceback logs
remote:Enumerating objects: 13, done.remote:Counting objects:   7% (1/13)remote:Counting objects:  15% (2/13)remote:Counting objects:  23% (3/13)remote:Counting objects:  30% (4/13)remote:Counting objects:  38% (5/13)remote:Counting objects:  46% (6/13)remote:Counting objects:  53% (7/13)remote:Counting objects:  61% (8/13)remote:Counting objects:  69% (9/13)remote:Counting objects:  76% (10/13)remote:Counting objects:  84% (11/13)remote:Counting objects:  92% (12/13)remote:Counting objects: 100% (13/13)remote:Counting objects: 100% (13/13), done.remote:Compressing objects:   7% (1/13)remote:Compressing objects:  15% (2/13)remote:Compressing objects:  23% (3/13)remote:Compressing objects:  30% (4/13)remote:Compressing objects:  38% (5/13)remote:Compressing objects:  46% (6/13)remote:Compressing objects:  53% (7/13)remote:Compressing objects:  61% (8/13)remote:Compressing objects:  69% (9/13)remote:Compressing objects:  76% (10/13)remote:Compressing objects:  84% (11/13)remote:Compressing objects:  92% (12/13)remote:Compressing objects: 100% (13/13)remote:Compressing objects: 100% (13/13), done.remote:Total 13 (delta 0), reused 3 (delta 0), pack-reused 0 (from 0)From https://github.com/python/cpython * branch                    main       -> FETCH_HEADNote:switching to '1503fc8f88d4903e61f76a78a30bcd581b0ee0cd'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by switching back to a branch.If you want to create a new branch to retain commits you create, you maydo so (now or later) by using -c with the switch command. Example:  git switch -c <new-branch-name>Or undo this operation with:  git switch -Turn off this advice by setting config variable advice.detachedHead to falseHEAD is now at 1503fc8f88d gh-127610: Added validation for more than one var-positional and var-keyword parameters in inspect.Signature (GH-127657)Switched to and reset branch 'main'configure:WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)configure:WARNING: pkg-config is missing. Some dependencies may not be detected correctly.In file included from ../../Python/ceval.c:904:../../Python/generated_cases.c.h:809:31: warning: code will never be executed [-Wunreachable-code]for (int _i = oparg;--_i>=0;) {^~~~~../../Python/generated_cases.c.h:695:31: warning: code will never be executed [-Wunreachable-code]for (int _i = oparg*2;--_i>=0;) {^~~~~2 warnings generated.  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0100  192k  100  192k    0     0  1360k      0 --:--:-- --:--:-- --:--:-- 1360k  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0100 42455  100 42455    0     0   327k      0 --:--:-- --:--:-- --:--:--  327k  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0100 5041k  100 5041k    0     0  29.2M      0 --:--:-- --:--:-- --:--:-- 29.2M  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0100  947k  100  947k    0     0  7928k      0 --:--:-- --:--:-- --:--:-- 7928k  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current                                 Dload  Upload   Total   Spent    Left  Speed  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0  0   544    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0curl:(22) The requested URL returned error: 403

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotiOS ARM64 Simulator 3.x has failed when building commit1503fc8.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1380/builds/2063) and take a look at the build logs.
  4. Check if the failure is related to this commit (1503fc8) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1380/builds/2063

Summary of the results of the build (if available):

Click to see traceback logs
remote:Enumerating objects: 13, done.remote:Counting objects:   7% (1/13)remote:Counting objects:  15% (2/13)remote:Counting objects:  23% (3/13)remote:Counting objects:  30% (4/13)remote:Counting objects:  38% (5/13)remote:Counting objects:  46% (6/13)remote:Counting objects:  53% (7/13)remote:Counting objects:  61% (8/13)remote:Counting objects:  69% (9/13)remote:Counting objects:  76% (10/13)remote:Counting objects:  84% (11/13)remote:Counting objects:  92% (12/13)remote:Counting objects: 100% (13/13)remote:Counting objects: 100% (13/13), done.remote:Compressing objects:   7% (1/13)remote:Compressing objects:  15% (2/13)remote:Compressing objects:  23% (3/13)remote:Compressing objects:  30% (4/13)remote:Compressing objects:  38% (5/13)remote:Compressing objects:  46% (6/13)remote:Compressing objects:  53% (7/13)remote:Compressing objects:  61% (8/13)remote:Compressing objects:  69% (9/13)remote:Compressing objects:  76% (10/13)remote:Compressing objects:  84% (11/13)remote:Compressing objects:  92% (12/13)remote:Compressing objects: 100% (13/13)remote:Compressing objects: 100% (13/13), done.remote:Total 13 (delta 0), reused 3 (delta 0), pack-reused 0 (from 0)From https://github.com/python/cpython * branch                    main       -> FETCH_HEADNote:switching to '1503fc8f88d4903e61f76a78a30bcd581b0ee0cd'.You are in 'detached HEAD' state. You can look around, make experimentalchanges and commit them, and you can discard any commits you make in thisstate without impacting any branches by switching back to a branch.If you want to create a new branch to retain commits you create, you maydo so (now or later) by using -c with the switch command. Example:  git switch -c <new-branch-name>Or undo this operation with:  git switch -Turn off this advice by setting config variable advice.detachedHead to falseHEAD is now at 1503fc8f88d gh-127610: Added validation for more than one var-positional and var-keyword parameters in inspect.Signature (GH-127657)Switched to and reset branch 'main'configure:WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)configure:WARNING: pkg-config is missing. Some dependencies may not be detected correctly.In file included from ../../Python/ceval.c:904:../../Python/generated_cases.c.h:809:31: warning: code will never be executed [-Wunreachable-code]for (int _i = oparg;--_i>=0;) {^~~~~../../Python/generated_cases.c.h:695:31: warning: code will never be executed [-Wunreachable-code]for (int _i = oparg*2;--_i>=0;) {^~~~~2 warnings generated.configure:WARNING: no system libmpdecimal found; falling back to bundled libmpdecimal (deprecated and scheduled for removal in Python 3.15)configure:WARNING: pkg-config is missing. Some dependencies may not be detected correctly.--- xcodebuild: WARNING: Using the first of multiple matching destinations:{ platform:iOS Simulator, id:C6AEA11C-C34A-47B8-BD67-AF0403ECA353, OS:17.5, name:iPhone SE (3rd generation) }{ platform:iOS Simulator, id:C6AEA11C-C34A-47B8-BD67-AF0403ECA353, OS:17.5, name:iPhone SE (3rd generation) }2024-12-08 10:27:40.461 xcodebuild[5085:442247520] [MT] IDETestOperationsObserverDebug: 1028.103 elapsed -- Testing started completed.2024-12-08 10:27:40.461 xcodebuild[5085:442247520] [MT] IDETestOperationsObserverDebug: 0.000 sec, +0.000 sec -- start2024-12-08 10:27:40.461 xcodebuild[5085:442247520] [MT] IDETestOperationsObserverDebug: 1028.103 sec, +1028.103 sec -- endFailing tests:-[iOSTestbedTests testPython]** TEST FAILED **make:*** [testios] Error 1

@Eclips4
Copy link
Member

Eclips4 commentedDec 8, 2024•
edited
Loading

@serhiy-storchaka what do you think about backports to 3.13 and 3.12?

FYI: buildbots failures are unrelated, it seems that there is some kind of network issue.

@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure⚠️⚠️⚠️

Hi! The buildbotAMD64 Arch Linux Perf 3.x has failed when building commit1503fc8.

What do you need to do:

  1. Don't panic.
  2. Checkthe buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/#/builders/1078/builds/4005) and take a look at the build logs.
  4. Check if the failure is related to this commit (1503fc8) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/#/builders/1078/builds/4005

Summary of the results of the build (if available):

==

Click to see traceback logs
Traceback (most recent call last):  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/threading.py", line1050, in_bootstrap_innerself.run()~~~~~~~~^^  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/test/test_poplib.py", line235, inrun    asyncore.loop(timeout=0.1,count=1)~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/test/support/asyncore.py", line213, inloop    poll_fun(timeout,map)~~~~~~~~^^^^^^^^^^^^^^  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/test/support/asyncore.py", line156, inpoll    read(obj)~~~~^^^^^  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/test/support/asyncore.py", line93, inread    obj.handle_error()~~~~~~~~~~~~~~~~^^  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/test/support/asyncore.py", line89, inread    obj.handle_read_event()~~~~~~~~~~~~~~~~~~~~~^^  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/test/support/asyncore.py", line426, inhandle_read_eventself.handle_read()~~~~~~~~~~~~~~~~^^  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/test/test_poplib.py", line203, inhandle_read    asynchat.async_chat.handle_read(self)~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/test/support/asynchat.py", line128, inhandle_readself.handle_error()~~~~~~~~~~~~~~~~~^^  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/test/support/asynchat.py", line124, inhandle_read    data=self.recv(self.ac_in_buffer_size)  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/test/support/asyncore.py", line377, inrecv    data=self.socket.recv(buffer_size)  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/ssl.py", line1285, inrecvreturnself.read(buflen)~~~~~~~~~^^^^^^^^  File"/buildbot/buildarea/3.x.pablogsal-arch-x86_64.perfbuild/build/Lib/ssl.py", line1140, inreadreturnself._sslobj.read(len)~~~~~~~~~~~~~~~~~^^^^^ssl.SSLError:[SYS] unknown error (_ssl.c:2645)k

srinivasreddy pushed a commit to srinivasreddy/cpython that referenced this pull requestJan 8, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@skirpichevskirpichevskirpichev approved these changes

@serhiy-storchakaserhiy-storchakaserhiy-storchaka approved these changes

@picnixzpicnixzpicnixz approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@ApostolFet@bedevere-bot@Eclips4@skirpichev@serhiy-storchaka@picnixz
[8]ページ先頭
©2009-2025 Movatter.jp