bpo-33197: Update error message of invalid inspect.Parameters #6636

corona10 · 2018-04-29T15:00:56Z

The error message from constructing invalid inspect.
Parameters was confused due to print integer enum value (parameter kind).
It is updated to print a string value of parameter kind.

https://bugs.python.org/issue33197

corona10 · 2018-04-29T15:05:24Z

For reviewers,

Please add labels needs backport to 3.7/3.6

serhiy-storchaka

kind can be not only _ParameterKind member here, but an integer. You need first make a _ParameterKind from it: _ParameterKind(kind).

There is the same formatting error at line 2465. The message at line 2739 can be improved too.

The message at line 2456 should contain the name of the type instead of the repr of the value.

bedevere-bot · 2018-04-29T15:56:33Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be poked with soft cushions!

corona10 · 2018-04-30T02:29:53Z

@serhiy-storchaka
Thenk you for kind review, I will reflect your review soon.

corona10 · 2018-04-30T09:14:43Z

I have made the requested changes; please review again

bedevere-bot · 2018-04-30T09:14:45Z

Thanks for making the requested changes!

@serhiy-storchaka: please review the changes made to this pull request.

1st1

This is a regression from blindly converting everything in stdlib to use enums. Thanks for fixing it!

I don't like the 'VAR_KEYWORD cannot have default values' message, it's rather cryptic. Please add a mapping of parameter-kinds to human-readable form and use that mapping to render good-looking error messages. Thanks!

1st1 · 2018-04-30T20:04:30Z

Lib/inspect.py

-            if kind in (_VAR_POSITIONAL, _VAR_KEYWORD):
-                msg = '{} parameters cannot have default values'.format(kind)
+            if self._kind in (_VAR_POSITIONAL, _VAR_KEYWORD):
+                msg = f'{self._kind!s} cannot have default values'


Why did you remove 'parameters' word from the message?

@1st1
No reason for it, I revert it.

bedevere-bot · 2018-04-30T20:05:59Z

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

corona10 · 2018-05-01T15:29:51Z

@1st1
Updated!

corona10 · 2018-05-25T09:35:23Z

@1st1, @serhiy-storchaka: Updated!

I will send a new PR about adding property after this PR is merged!(3.8)

corona10 · 2018-05-26T16:00:08Z

@1st1

Please take a look if you have a time 👍

1st1 · 2018-05-26T16:08:51Z

Lib/test/test_inspect.py

@@ -1444,6 +1444,19 @@ def f6(a, b, c):
        with self.assertRaisesRegex(TypeError, "'a', 'b' and 'c'"):
            inspect.getcallargs(f6)

+        # bpo-33197
+        with self.assertRaisesRegex(ValueError,
+                                    'variadic keyword parameters cannot have default values'):


1st1 · 2018-05-26T16:09:17Z

Lib/test/test_inspect.py

+            inspect.Parameter("foo", kind=inspect.Parameter.VAR_KEYWORD,
+                              default=42)
+        with self.assertRaisesRegex(ValueError,
+                                    "invalid value for 'Parameter.kind' attribute"):


corona10 · 2018-05-26T20:56:17Z

@1st1

Thanks you for reviewing all 79 characters linting issues.
Now there are no longer characters over 79 characters on this patch
including inspect.py, test_inspect.py and news.

I should care about this linting convention from now on.

Thank you for redundunt reviewing for same issues :)

Enjoy your weekend! thank you

1st1 · 2018-05-27T17:01:56Z

Lib/inspect.py

+            self._kind = _ParameterKind(kind)
+        except ValueError:
+            msg = "invalid value for 'Parameter.kind' attribute"
+            raise ValueError(msg) from None


last change here:

raise ValueError(f'value {kind!r} is not a valid Parameter.kind')

1st1 · 2018-05-27T17:04:06Z

Lib/inspect.py

+            if self._kind != _POSITIONAL_OR_KEYWORD:
+                msg = (
+                    'implicit arguments must be passed in as '
+                    '{} arguments not {} arguments'


"implicit arguments must be passed as positional or keyword arguments, not {}"

1st1

Left a couple nitpicks, please fix them. LGTM otherwise.

The error message from constructing invalid inspect. Parameters was confused due to print integer enum value (parameter kind). It is updated to print a string value of parameter kind.

corona10 · 2018-05-28T02:30:25Z

@1st1 @serhiy-storchaka

Thanks! Updated!

miss-islington · 2018-05-29T15:04:11Z

Thanks @corona10 for the PR, and @1st1 for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

1st1 · 2018-05-29T15:04:20Z

Thanks, @corona10

bedevere-bot · 2018-05-29T15:04:31Z

GH-7204 is a backport of this pull request to the 3.7 branch.

…honGH-6636) (cherry picked from commit a9cab43) Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

bedevere-bot · 2018-05-29T15:05:21Z

GH-7205 is a backport of this pull request to the 3.6 branch.

…6636) (GH-7204) (cherry picked from commit a9cab43) Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

…6636) (#7205) (cherry picked from commit a9cab43) Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

bedevere-bot added the awaiting review label Apr 29, 2018

corona10 force-pushed the bpo-33197 branch 2 times, most recently from a226596 to 67478b6 Compare April 29, 2018 15:22

serhiy-storchaka requested changes Apr 29, 2018

View reviewed changes

bedevere-bot removed the awaiting review label Apr 29, 2018

bedevere-bot added the awaiting changes label Apr 29, 2018

serhiy-storchaka added type-bug An unexpected behavior, bug, or error needs backport to 3.6 labels Apr 29, 2018

corona10 force-pushed the bpo-33197 branch from 67478b6 to 4a25eb8 Compare April 30, 2018 06:27

corona10 changed the title ~~bpo-33197: Update error message of invalid inspect.Parameters~~ [WIP] bpo-33197: Update error message of invalid inspect.Parameters Apr 30, 2018

corona10 force-pushed the bpo-33197 branch 2 times, most recently from 96620b3 to 4fb6fca Compare April 30, 2018 08:23

corona10 changed the title ~~[WIP] bpo-33197: Update error message of invalid inspect.Parameters~~ bpo-33197: Update error message of invalid inspect.Parameters Apr 30, 2018

bedevere-bot added awaiting change review and removed awaiting changes labels Apr 30, 2018

1st1 requested changes Apr 30, 2018

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting change review labels Apr 30, 2018

corona10 force-pushed the bpo-33197 branch from 4fb6fca to 179550f Compare May 1, 2018 02:28

corona10 changed the title ~~bpo-33197: Update error message of invalid inspect.Parameters~~ [WIP]bpo-33197: Update error message of invalid inspect.Parameters May 1, 2018

corona10 force-pushed the bpo-33197 branch from 179550f to e0372d2 Compare May 1, 2018 15:16

corona10 changed the title ~~[WIP]bpo-33197: Update error message of invalid inspect.Parameters~~ bpo-33197: Update error message of invalid inspect.Parameters May 1, 2018

1st1 reviewed May 26, 2018

View reviewed changes

corona10 force-pushed the bpo-33197 branch from 861f625 to e1ad1cd Compare May 26, 2018 20:18

1st1 reviewed May 27, 2018

View reviewed changes

1st1 approved these changes May 27, 2018

View reviewed changes

corona10 force-pushed the bpo-33197 branch 2 times, most recently from d6dde9d to a466687 Compare May 27, 2018 23:58

bpo-33197: Update a error message of invalid inspect.Parameters.

355f1ac

The error message from constructing invalid inspect. Parameters was confused due to print integer enum value (parameter kind). It is updated to print a string value of parameter kind.

corona10 force-pushed the bpo-33197 branch from a466687 to 355f1ac Compare May 28, 2018 01:27

1st1 approved these changes May 29, 2018

View reviewed changes

1st1 merged commit a9cab43 into python:master May 29, 2018

bedevere-bot removed the awaiting merge label May 29, 2018

bedevere-bot removed the needs backport to 3.7 label May 29, 2018

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 29, 2018

bpo-33197: Update a error message of invalid inspect.Parameters. (pyt…

bf71b04

…honGH-6636) (cherry picked from commit a9cab43) Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 29, 2018

bpo-33197: Update a error message of invalid inspect.Parameters. (pyt…

3ad4088

…honGH-6636) (cherry picked from commit a9cab43) Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

bedevere-bot removed the needs backport to 3.6 label May 29, 2018

corona10 mentioned this pull request May 29, 2018

bpo-33197: Add description property for _ParameterKind #7206

Merged

1st1 pushed a commit that referenced this pull request May 29, 2018

bpo-33197: Update a error message of invalid inspect.Parameters. (GH-…

cb055bc

…6636) (GH-7204) (cherry picked from commit a9cab43) Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

1st1 pushed a commit that referenced this pull request May 29, 2018

bpo-33197: Update a error message of invalid inspect.Parameters. (GH-…

5e9e9db

…6636) (#7205) (cherry picked from commit a9cab43) Co-authored-by: Dong-hee Na <donghee.na92@gmail.com>

corona10 deleted the bpo-33197 branch June 10, 2018 08:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bpo-33197: Update error message of invalid inspect.Parameters #6636

bpo-33197: Update error message of invalid inspect.Parameters #6636

corona10 commented Apr 29, 2018 •

edited by bedevere-bot

Loading

corona10 commented Apr 29, 2018

serhiy-storchaka left a comment

bedevere-bot commented Apr 29, 2018

corona10 commented Apr 30, 2018

corona10 commented Apr 30, 2018

bedevere-bot commented Apr 30, 2018

1st1 left a comment

1st1 Apr 30, 2018

corona10 May 1, 2018 •

edited

Loading

bedevere-bot commented Apr 30, 2018

corona10 commented May 1, 2018

corona10 commented May 25, 2018 •

edited

Loading

corona10 commented May 26, 2018

1st1 May 26, 2018

1st1 May 26, 2018

corona10 commented May 26, 2018

1st1 May 27, 2018

1st1 May 27, 2018

1st1 left a comment

corona10 commented May 28, 2018

miss-islington commented May 29, 2018

1st1 commented May 29, 2018

bedevere-bot commented May 29, 2018

bedevere-bot commented May 29, 2018

bpo-33197: Update error message of invalid inspect.Parameters #6636

bpo-33197: Update error message of invalid inspect.Parameters #6636

Conversation

corona10 commented Apr 29, 2018 • edited by bedevere-bot Loading

corona10 commented Apr 29, 2018

serhiy-storchaka left a comment

Choose a reason for hiding this comment

bedevere-bot commented Apr 29, 2018

corona10 commented Apr 30, 2018

corona10 commented Apr 30, 2018

bedevere-bot commented Apr 30, 2018

1st1 left a comment

Choose a reason for hiding this comment

1st1 Apr 30, 2018

Choose a reason for hiding this comment

corona10 May 1, 2018 • edited Loading

Choose a reason for hiding this comment

bedevere-bot commented Apr 30, 2018

corona10 commented May 1, 2018

corona10 commented May 25, 2018 • edited Loading

corona10 commented May 26, 2018

1st1 May 26, 2018

Choose a reason for hiding this comment

1st1 May 26, 2018

Choose a reason for hiding this comment

corona10 commented May 26, 2018

1st1 May 27, 2018

Choose a reason for hiding this comment

1st1 May 27, 2018

Choose a reason for hiding this comment

1st1 left a comment

Choose a reason for hiding this comment

corona10 commented May 28, 2018

miss-islington commented May 29, 2018

1st1 commented May 29, 2018

bedevere-bot commented May 29, 2018

bedevere-bot commented May 29, 2018

corona10 commented Apr 29, 2018 •

edited by bedevere-bot

Loading

corona10 May 1, 2018 •

edited

Loading

corona10 commented May 25, 2018 •

edited

Loading