-
-
Notifications
You must be signed in to change notification settings - Fork 32.2k
gh-130425: Add "Did you mean" suggestion for del 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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
I agree the tests should be added, a separate issue would be good :-) |
Misc/NEWS.d/next/Core_and_Builtins/2025-02-22-01-23-23.gh-issue-130425.x5SNQ8.rst
Outdated
Show resolved
Hide resolved
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!
…gestions-combined This merges the delattr suggestion feature implementation by @sobolevn from PR python#130427 with added tests from PR python#130455.
…to delattr-suggestions-combined Adds test cases for the delattr suggestion feature implemented in PR python#130427 by @sobolevn.
I think that the PR is quite obvious and combining it with tests is not really worth it :( |
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. |
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). |
Ok, then - let's wait for #130799 to get finished |
This was a really simple PR that collapsed under the complexity of the review process. Closing. There are now 2 alternative PRs. |
I am not quite sure about the proper way to test this:
delattr
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