GeezerVison

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

GeezerVison is showing github.com — gh-113978: Ignore warnings on text completion inside REPL by WolframAlph · Pull Request #113979 · python/cpython

Conversation

WolframAlph commented Jan 12, 2024

Copy link

Copy Markdown

Contributor

gh-113978

Sorry, something went wrong.

Uh oh!

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

All reactions


          pythongh-113978: Ignore warnings on text completion inside REPL

7fe3c23

bedevere-app Bot added the awaiting review label

Jan 12, 2024

bedevere-app Bot mentioned this pull request

Jan 12, 2024

Deprecation warning during text completion in REPL #113978

Closed

sobolevn reviewed

Jan 12, 2024

View reviewed changes

sobolevn 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.

Maybe instead we should do this?

» git patch
diff --git Lib/rlcompleter.py Lib/rlcompleter.py
index 206d6fb511c..62fd2c7fdec 100644
--- Lib/rlcompleter.py
+++ Lib/rlcompleter.py
@@ -176,8 +176,9 @@ def attr_matches(self, text):
                 if (word[:n] == attr and
                     not (noprefix and word[:n+1] == noprefix)):
                     match = "%s.%s" % (expr, word)
+                    import types
                     if isinstance(getattr(type(thisobject), word, None),
-                                  property):
+                                  (property, types.GetSetDescriptorType)):
                         # bpo-44752: thisobject.word is a method decorated by
                         # `@property`. What follows applies a postfix if
                         # thisobject.word is callable, but know we know that

It also solves the issue:

Sorry, something went wrong.

Uh oh!

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

All reactions

WolframAlph commented Jan 12, 2024

Copy link

Copy Markdown

Contributor Author

Not really sure. My intention was to eliminate all possible warnings during completion (now and in future) no matter where they come from. Maybe it's the only possible path where it can happen but I am not sure since I never dealt with such things. Your solution looks cleaner to me and I believe you have broader context so I leave it up to you to decide.

All reactions

Sorry, something went wrong.

Uh oh!

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

sobolevn commented Jan 12, 2024

Copy link

Copy Markdown

Member

My thoughs: properties should not evaluate when auto-complete works, so I think that adding an ignore just hides this problem.

So, we should really address the root of this problem: do not execute .co_lnotab property on code objects (and similar types as well).

sunmy2019 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.

WolframAlph commented Jan 12, 2024 •

edited

Loading

Uh oh!

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

Copy link

Copy Markdown

Contributor Author

properties should not evaluate when auto-complete works

I agree. But is this true only for built-in objects and their properties or user defined as well?
Consider following piece of code:

class D:
    def __get__(self, _, __):
        print("evaluated")

class C:
    prop = D()

trying to get suggestions for C. evaluates prop and prints the message.

All reactions

Sorry, something went wrong.

Uh oh!

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

sobolevn commented Jan 12, 2024

Copy link

Copy Markdown

Member

We can also try to replace getattr with inspect.getattr_static

All reactions

Sorry, something went wrong.

Uh oh!

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

WolframAlph commented Jan 14, 2024

Copy link

Copy Markdown

Contributor Author

Some things depend on being evaluated. Using inspect.getattr_static instead of getattr breaks tests.

cpython/Lib/test/test_rlcompleter.py

Lines 51 to 85 in 77b45fa

	def test_attr_matches(self):
	# test with builtins namespace
	self.assertEqual(self.stdcompleter.attr_matches('str.s'),
	['str.{}('.format(x) for x in dir(str)
	if x.startswith('s')])
	self.assertEqual(self.stdcompleter.attr_matches('tuple.foospamegg'), [])
	expected = sorted({'None.%s%s' % (x,
	'()' if x == '__init_subclass__'
	else '' if x == '__doc__'
	else '(')
	for x in dir(None)})
	self.assertEqual(self.stdcompleter.attr_matches('None.'), expected)
	self.assertEqual(self.stdcompleter.attr_matches('None._'), expected)
	self.assertEqual(self.stdcompleter.attr_matches('None.__'), expected)

	# test with a customized namespace
	self.assertEqual(self.completer.attr_matches('CompleteMe.sp'),
	['CompleteMe.spam'])
	self.assertEqual(self.completer.attr_matches('Completeme.egg'), [])
	self.assertEqual(self.completer.attr_matches('CompleteMe.'),
	['CompleteMe.mro()', 'CompleteMe.spam'])
	self.assertEqual(self.completer.attr_matches('CompleteMe._'),
	['CompleteMe._ham'])
	matches = self.completer.attr_matches('CompleteMe.__')
	for x in matches:
	self.assertTrue(x.startswith('CompleteMe.__'), x)
	self.assertIn('CompleteMe.__name__', matches)
	self.assertIn('CompleteMe.__new__(', matches)

	with patch.object(CompleteMe, "me", CompleteMe, create=True):
	self.assertEqual(self.completer.attr_matches('CompleteMe.me.me.sp'),
	['CompleteMe.me.me.spam'])
	self.assertEqual(self.completer.attr_matches('egg.s'),
	['egg.{}('.format(x) for x in dir(str)
	if x.startswith('s')])

Things like __init_subclass__, __class__, mro, etc...

All reactions

Sorry, something went wrong.

Uh oh!

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

ambv merged commit e03dde5 into python:main

May 21, 2024

bedevere-app Bot removed the awaiting review label

May 21, 2024

lysnikolaou added topic-repl

Related to the interactive shell

needs backport to 3.13 only security fixes labels

May 22, 2024

miss-islington-app Bot commented May 22, 2024

Copy link

Copy Markdown

Thanks @WolframAlph for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request

May 22, 2024


          pythongh-113978: Ignore warnings on text completion inside REPL (pyth…

66f7373

…onGH-113979)

(cherry picked from commit e03dde5)

Co-authored-by: Yan Yanchii <[email protected]>

bedevere-app Bot commented May 22, 2024

Copy link

Copy Markdown

GH-119429 is a backport of this pull request to the 3.13 branch.

All reactions

Sorry, something went wrong.

Uh oh!

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

bedevere-app Bot removed the needs backport to 3.13 only security fixes label

May 22, 2024

ambv pushed a commit that referenced this pull request

May 22, 2024


          [3.13] gh-113978: Ignore warnings on text completion inside REPL (GH-…

6bc7fc0

…113979) (#119429)

(cherry picked from commit e03dde5)

Co-authored-by: Yan Yanchii <[email protected]>

JelleZijlstra mentioned this pull request

May 28, 2024

backport a9a74da 3.13 #119642

Closed

estyxx pushed a commit to estyxx/cpython that referenced this pull request

Jul 17, 2024


          pythongh-113978: Ignore warnings on text completion inside REPL (pyth…

2f7af8b

…on#113979)

Reviewers

sobolevn

sobolevn left review comments

Uh oh!

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

Assignees

No one assigned

Labels

topic-repl

Related to the interactive shell

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

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.

GeezerVison

Conversation

WolframAlph commented Jan 12, 2024

Uh oh!

sobolevn left a comment

Choose a reason for hiding this comment

Uh oh!

WolframAlph commented Jan 12, 2024

Uh oh!

sobolevn commented Jan 12, 2024

Uh oh!

WolframAlph commented Jan 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sobolevn commented Jan 12, 2024

Uh oh!

WolframAlph commented Jan 14, 2024

Uh oh!

miss-islington-app Bot commented May 22, 2024

Uh oh!

bedevere-app Bot commented May 22, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

WolframAlph commented Jan 12, 2024 •

edited

Loading

Uh oh!

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