GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-130425: Add "Did you mean" suggestion for `del obj.attr` by sobolevn · Pull Request #130427 · python/cpython

Conversation

@sobolevn

@sobolevn sobolevn commented Feb 21, 2025 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

  • There are no tests for delattr and setattr 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.exe     
Python 3.14.0a5+ (heads/main-dirty:38642bff139, Feb 22 2025, 01:18:34) [Clang 15.0.0 (clang-1500.3.9.4)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> class A:
...     pass
... 
... a = A()
... a.abcde = 1
... del a.abcdf # typo
... 
Traceback (most recent call last):
  File "<python-input-0>", line 6, in <module>
    del a.abcdf # typo
        ^^^^^^^
AttributeError: 'A' object has no attribute 'abcdf'. Did you mean: 'abcde'?
>>>
  • Issue: Provide name suggestions for failed attribute deletions #130425

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@sobolevn
pythongh-130425: Add "Did you mean" suggestion for del obj.attr
1177f20
@sobolevn
sobolevn requested review from ambv and pablogsal February 21, 2025 22:25
@sobolevn
sobolevn requested review from markshannon and methane as code owners February 21, 2025 22:25
@bedevere-app bedevere-app Bot added the awaiting core review label Feb 21, 2025
@bedevere-app bedevere-app Bot mentioned this pull request Feb 21, 2025
Provide name suggestions for failed attribute deletions #130425
Closed
@StanFromIreland

StanFromIreland commented Feb 21, 2025

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
StanFromIreland
StanFromIreland reviewed Feb 21, 2025
View reviewed changes
Comment thread Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-01-23-23.gh-issue-130425.x5SNQ8.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn
Update NEWS
818decc
StanFromIreland
StanFromIreland approved these changes Feb 22, 2025
View reviewed changes

@StanFromIreland StanFromIreland left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Looks good now!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Pranjal095 Pranjal095 mentioned this pull request Feb 23, 2025
gh-130428: Add tests for delattr suggestions #130455
Closed
Pranjal095 added a commit to Pranjal095/cpython that referenced this pull request Mar 3, 2025
@Pranjal095
Merge remote-tracking branch 'sobolevn/issue-130425' into delattr-sug…
d9ceb27 
…gestions-combined

This merges the delattr suggestion feature implementation by @sobolevn from PR python#130427 with added tests from PR python#130455.
Pranjal095 added a commit to Pranjal095/cpython that referenced this pull request Mar 3, 2025
@Pranjal095
Merge remote-tracking branch 'Pranjal095/delattr-suggestion-tests' in…
2538806 
…to delattr-suggestions-combined

Adds test cases for the delattr suggestion feature implemented in PR python#130427 by @sobolevn.
@Pranjal095 Pranjal095 mentioned this pull request Mar 3, 2025
gh-130425: Add "Did you mean [...]" suggestions for del obj.attr #130799
Closed
@picnixz

picnixz commented Mar 3, 2025

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn

sobolevn commented Mar 4, 2025

Copy link
Copy Markdown
Member Author

I think that the PR is quite obvious and combining it with tests is not really worth it :(
StanFromIreland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@picnixz

picnixz commented Mar 4, 2025

Copy link
Copy Markdown
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 emoji sobolevn reacted with eyes emoji
All reactions
  • 👍 1 reaction
  • 👀 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@cfbolz

cfbolz commented Mar 4, 2025

Copy link
Copy Markdown
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
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn

sobolevn commented Mar 4, 2025

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn

sobolevn commented May 2, 2025

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@sobolevn sobolevn closed this May 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@StanFromIreland StanFromIreland StanFromIreland approved these changes
@ambv ambv Awaiting requested review from ambv
@pablogsal pablogsal Awaiting requested review from pablogsal
@methane methane Awaiting requested review from methane methane is a code owner
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

awaiting core review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@sobolevn @StanFromIreland @picnixz @cfbolz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.