Skip to content
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

Allow safe access to the __getattribute__ method of modules #14029

Merged
merged 2 commits into from
Apr 28, 2023

Conversation

ccordoba12
Copy link
Member

@ccordoba12 ccordoba12 commented Apr 22, 2023

This restores code completions of second-level modules when Jedi completions are disabled.

Fixes #14028

@ccordoba12
Copy link
Member Author

@krassowski, I don't understand very well how to add a test for this, so I'd appreciate your guidance for that.

This restores code completions of second-level modules when Jedi
completions are disabled.
@krassowski
Copy link
Member

Maybe something like:

def test_module_access():
    import sys
    context = limited(sys=sys)
    assert guarded_eval("sys.copyright", context) == sys.copyright

    context = minimal(sys=sys)
    with pytest.raises(GuardRejection):
        guarded_eval("sys.copyright", context)

in IPython/core/tests/test_guarded_eval.py?

@ccordoba12
Copy link
Member Author

ccordoba12 commented Apr 22, 2023

Thanks @krassowski! I implemented the same test but using Numpy because I think it's a more important use case.

Copy link
Member

@krassowski krassowski left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @ccordoba12!

Comment on lines 639 to +640
method_descriptor: Any = type(list.copy)
module = type(builtins)
Copy link
Member

Choose a reason for hiding this comment

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

Hum,

I missed that in the previous PR but those should likely be :

from types import MethodDescriptorType
from types import ModuleType

Not blocking so let's open as another issue.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, MethodDescriptorType is neat. I missed it being added in 3.7, but since we are not supporting 3.6 for some time now this is good idea indeed.

Copy link
Member

Choose a reason for hiding this comment

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

Technically we don't even support 3.8 anymore, so plenty of new things you can use !

Copy link
Member Author

Choose a reason for hiding this comment

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

I can take care of this.

@Carreau Carreau merged commit c57c70c into ipython:main Apr 28, 2023
22 checks passed
@ccordoba12 ccordoba12 deleted the issue-14028 branch April 28, 2023 13:41
@ccordoba12
Copy link
Member Author

ccordoba12 commented Apr 28, 2023

@Carreau, could this fix be backported to the 8.12 branch? We still have a lot of people that uses Python 3.8 in Spyder, so we'd like them to benefit from it.

@Carreau Carreau modified the milestones: 8.13, 8.12.2 May 2, 2023
@Carreau
Copy link
Member

Carreau commented May 2, 2023

meeseeksdev backport to 8.12.x

meeseeksmachine pushed a commit to meeseeksmachine/ipython that referenced this pull request May 2, 2023
Carreau added a commit that referenced this pull request May 4, 2023
…ttribute__` method of modules) (#14065)

Backport PR #14029: Allow safe access to the `__getattribute__` method
of modules
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.

IPython can't provide completions for second level modules when Jedi completions are disabled
3 participants