FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

gh-115937: remove extra processing for the __signature__ attribute by skirpichev · Pull Request #115984 · python/cpython · GitHub

  1. python
  2. cpython

Repository navigation

gh-115937: remove extra processing for the __signature__ attribute - #115984

Closed
skirpichev wants to merge 1 commit into
python:mainfrom
skirpichev:enum-signature-115937
ConversationCommits (1)ChecksFiles changed
Closed

gh-115937: remove extra processing for the __signature__ attribute#115984
skirpichev wants to merge 1 commit into
python:mainfrom
skirpichev:enum-signature-115937

Conversation

skirpichev commented Feb 27, 2024 •
edited by bedevere-app Bot
Loading

Copy link
Copy Markdown
Member

bedevere-app Bot mentioned this pull request Feb 27, 2024
Closed
Gouvernathor reviewed Feb 27, 2024
View reviewed changes
Comment thread Lib/test/test_inspect/test_inspect.py
((('a', ..., ..., 'positional_or_keyword'),
('b', 2, ..., 'positional_or_keyword')),
...))

Gouvernathor Feb 27, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think this should test that random objects should be passed as-is by the signature function (5, None, []...).
But maybe making this implementation perfect is not urgent as long as a decision has not been made as to what follow-up to bring to the base issue.

skirpichev Feb 27, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Since 42407ab there is also type check for returned sig object. So, my commit just returns things to pre-#100168 state (enum module tests kept).

Gouvernathor Feb 27, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ok, this is long-standing behavior, but then it needs to be documented. (again, it can wait)

Gouvernathor mentioned this pull request Feb 28, 2024
Merged

erlend-aasland commented Feb 29, 2024

Copy link
Copy Markdown
Contributor

Closing in favour of #116086. Thanks for your interest in improving CPython.

skirpichev deleted the enum-signature-115937 branch February 29, 2024 09:55

skirpichev commented Feb 29, 2024

Copy link
Copy Markdown
Member Author

Thanks for your interest in improving CPython.

Obviously, I'm not very good in this:)

erlend-aasland commented Feb 29, 2024

Copy link
Copy Markdown
Contributor

Obviously, I'm not very good in this:)

I think you've made a lot of very nice contributions, both code contributions, and as a participant in discussions. Sometimes we disagree, and that's ok. It is also often difficult to get a concensus within the core dev group. That's also ok.

skirpichev mentioned this pull request Feb 29, 2024
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

ethanfurman Awaiting requested review from ethanfurman ethanfurman is a code owner
1 more reviewer
Gouvernathor Gouvernathor left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

awaiting review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL