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-130425: Add "Did you mean" suggestion fordel obj.attr#130427

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

Closed
sobolevn wants to merge2 commits intopython:mainfromsobolevn:issue-130425

Conversation

sobolevn
Copy link
Member

@sobolevnsobolevn commentedFeb 21, 2025•
edited by bedevere-appbot
Loading

I am not quite sure about the proper way to test this:

  • There are no tests fordelattr andsetattr suggestions, it feels like this should be a separate issue to add them. I can do it in the next PR, there would be lots of corner cases
  • Local testing works:
» ./python.exePython3.14.0a5+ (heads/main-dirty:38642bff139,Feb222025,01:18:34) [Clang15.0.0 (clang-1500.3.9.4)]ondarwinType"help","copyright","credits"or"license"formoreinformation.>>>classA:...pass... ...a=A()...a.abcde=1...dela.abcdf# typo...Traceback (mostrecentcalllast):File"<python-input-0>",line6,in<module>dela.abcdf# typo^^^^^^^AttributeError:'A'objecthasnoattribute'abcdf'.Didyoumean:'abcde'?>>>

@StanFromIreland
Copy link
Contributor

I agree the tests should be added, a separate issue would be good :-)

Copy link
Contributor

@StanFromIrelandStanFromIreland left a comment

Choose a reason for hiding this comment

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

Looks good now!

Pranjal095 added a commit to Pranjal095/cpython that referenced this pull requestMar 3, 2025
…gestions-combinedThis merges the delattr suggestion feature implementation by@sobolevn from PRpython#130427 with added tests from PRpython#130455.
Pranjal095 added a commit to Pranjal095/cpython that referenced this pull requestMar 3, 2025
…to delattr-suggestions-combinedAdds test cases for the delattr suggestion feature implemented in PRpython#130427 by@sobolevn.
@picnixz
Copy link
Member

I would suggest either using this one and backport the tests from#130455 or use#130799 (which backports this PR).

@sobolevn
Copy link
MemberAuthor

I think that the PR is quite obvious and combining it with tests is not really worth it :(

StanFromIreland reacted with thumbs up emoji

@picnixz
Copy link
Member

Well@encukou actually asked for a merged PR. I would also prefer having both tests and implementation in the same commit so to avoid double commit reversion in case of an issue we didn't forsee.

encukou reacted with thumbs up emojisobolevn reacted with eyes emoji

@cfbolz
Copy link
Contributor

Please add tests. I agree that the implementation is really simple, but eg PyPy relies on the standard library tests a lot to find out what details in behaviour we are missing (and we already implemented delattr suggestions, so even more important in this specific case).

encukou reacted with thumbs up emoji

@sobolevn
Copy link
MemberAuthor

Ok, then - let's wait for#130799 to get finished

@sobolevn
Copy link
MemberAuthor

This was a really simple PR that collapsed under the complexity of the review process. Closing. There are now 2 alternative PRs.

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@StanFromIrelandStanFromIrelandStanFromIreland approved these changes

@ambvambvAwaiting requested review from ambv

@pablogsalpablogsalAwaiting requested review from pablogsal

@methanemethaneAwaiting requested review from methanemethane is a code owner

@markshannonmarkshannonAwaiting requested review from markshannonmarkshannon is a code owner

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@sobolevn@StanFromIreland@picnixz@cfbolz
[8]ページ先頭
©2009-2025 Movatter.jp