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

PR: Add a global filter flag to settings #460

Merged
merged 8 commits into from
Jul 12, 2023

Conversation

jsbautista
Copy link
Contributor

@jsbautista jsbautista commented Jun 13, 2023

@ccordoba12 ccordoba12 changed the title PR: Add filters flag PR: Add global filter flag Jun 13, 2023
@ccordoba12 ccordoba12 requested a review from dalthviz June 13, 2023 16:36
@dalthviz dalthviz added this to the v3.0.0b1 milestone Jun 13, 2023
@ccordoba12 ccordoba12 modified the milestones: v3.0.0b1, v3.0.0b2 Jun 14, 2023
@ccordoba12
Copy link
Member

@jsbautista or @dalthviz, could you add a test for this? It could be as simple as building the namespace view with and without the filter on and checking that a specific variable is absent/present in each case.

Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @jsbautista for the work here! Left a small suggestion. Regarding the test, I think we can use as boilerplate the one at:

def test_get_namespace_view(kernel):
"""
Test the namespace view of the kernel.
"""
execute = asyncio.run(kernel.do_execute('a = 1', True))
nsview = repr(kernel.get_namespace_view())
assert "'a':" in nsview
assert "'type': 'int'" in nsview or "'type': u'int'" in nsview
assert "'size': 1" in nsview
assert "'view': '1'" in nsview
assert "'numpy_type': 'Unknown'" in nsview
assert "'python_type': 'int'" in nsview

If you have any question let me know!

spyder_kernels/utils/nsview.py Outdated Show resolved Hide resolved
@jsbautista jsbautista requested a review from dalthviz July 11, 2023 19:34
@jsbautista jsbautista requested a review from dalthviz July 11, 2023 19:46
Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @jsbautista ! LGTM 👍

@dalthviz dalthviz requested a review from ccordoba12 July 11, 2023 19:52
Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Looks good to me now, thanks @jsbautista for your work on this!

@ccordoba12
Copy link
Member

@dalthviz, please squash-merge as soon as @jsbautista resolves your comment above.

@ccordoba12 ccordoba12 changed the title PR: Add global filter flag PR: Add a global filter flag to settings Jul 12, 2023
@dalthviz dalthviz merged commit 838f55d into spyder-ide:master Jul 12, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants