Skip to content

gh-130425: Add "Did you mean [...]" suggestions for del obj.attr #136588

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 23 commits into
base: main
Choose a base branch
from

Conversation

Pranjal095
Copy link

@Pranjal095 Pranjal095 commented Jul 12, 2025

This pull request replaces the earlier PR #130799 (now closed) and carries over that set of commits, preserving original authorship and history. An extra commit has been included to address a linting issue that was present in the previous version.

Issue: #130425

@picnixz picnixz changed the title gh-130425: Add "Did you mean [...]" suggestions for del obj.attr gh-130425: Add "Did you mean [...]" suggestions for del obj.attr Jul 12, 2025
@picnixz picnixz requested a review from sobolevn July 12, 2025 15:57
@Pranjal095 Pranjal095 requested a review from picnixz July 12, 2025 18:02
@Pranjal095 Pranjal095 requested a review from picnixz July 12, 2025 18:35
@Pranjal095 Pranjal095 requested a review from picnixz July 12, 2025 18:58
@StanFromIreland
Copy link
Member

@Pranjal095 Please mark comments as Resolved when you are finished with them, it helps keep the PR organized.

@Pranjal095 Pranjal095 requested a review from picnixz July 14, 2025 16:42
@Pranjal095 Pranjal095 requested a review from picnixz August 2, 2025 10:56
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

This looks good now.

I'll let @sobolevn have a look as well as he's credited as a contributor as well.

@Pranjal095 Pranjal095 requested a review from picnixz August 13, 2025 07:22
This allows moving test_suggestions_with_method_call back to
test_getattr_suggestions_underscored, so that delattr is tested
for these as well.
@encukou
Copy link
Member

encukou commented Aug 19, 2025

I noticed that some of the setattr tests don't run for delattr, but could. See Pranjal095#1

@Pranjal095 Pranjal095 requested a review from AA-Turner as a code owner August 19, 2025 09:18
@AA-Turner AA-Turner removed their request for review August 19, 2025 09:22
@Pranjal095
Copy link
Author

Thanks a lot for catching this! I hadn’t noticed the issue myself, and I’m very happy to have merged the changes.

Copy link
Member

@encukou encukou left a comment

Choose a reason for hiding this comment

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

LGTM

@sobolevn, I plan to merge around Friday if there are no objections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants