PR: Add a new plugin for the Pythonpath manager #19937

ccordoba12 · 2022-10-27T22:58:07Z

Description of Changes

This plugin simplifies how the Pythonpath set by users in Spyder is consumed by other plugins.
Add support to handle the PYTHONPATH env var set in the system through Spyder by detecting it, passing it to the IPython console, Completions and other plugins that make use of Spyder's Pythonpath, and allowing users to enable/disable their paths.
Add three headers to the QListWidget of our PathManager widget to break its contents into the following sections: project, user and system paths.
Place all buttons in PathManager to the right of its QListWidget to simplify its UI and avoid so much text on it.
Add a new on_close method to PluginMainContainer to run code when its closed.

Screenshots for PathManager:

Before

After

Issue(s) Resolved

Fixes #19724
Supersedes PR #19194
Supersedes PR #18778

Affirmation

By submitting this Pull Request or typing my (user)name below,
I affirm the Developer Certificate of Origin
with respect to all commits and content included in this PR,
and understand I am releasing the same under Spyder's MIT (Expat) license.

I certify the above statement is true and correct: @ccordoba12

There is no need to have them as such.

This breaks the path list into sections so that it's easier to understand where each path comes from.

This is an intermediate and unfinished commit because after working on this feature I realized that we really need a plugin to implement it in a simple and well structure way.

Also use spyder_pythonpath option from this plugin instead of the one stored in the main window settings.

…Pythonpath

Also fix test related to that functionality

Also use SpyderWidgetMixin API to create them and change their icons

mrclary · 2022-10-28T01:30:56Z

@ccordoba12, I've found that the enabled state of user paths does not persist to subsequent PPM instances:

However, quitting Spyder and relaunching shows the correct path enabled states. Also, the incorrect path enabled states do not propagate to IPython console's sys.path (good). So I suspect that this may be only an issue in the widget.

spyder/config/main.py

spyder/plugins/pythonpath/widgets/pathmanager.py

dalthviz

Thanks @ccordoba12 ! Checking locally on Windows I can see the following:

Seems like the export button is not aligned with the rest:
I can confirm @mrclary comment regarding the state of the paths activation not being changed when reopening the dialog (always shows the state as it was at startup).
After that, when closing and reopening Spyder for the first time, I didn't see the path that I added. I readded and I got the deactivated state correctly set. After that subsecuent Spyder sessions showed the path (not totally sure what happened here 🤔 )
Similarly with the state on the widget, also the order is not being shown properly (although the changes are actually applied):

mrclary · 2022-10-28T18:04:40Z

After that, when closing and reopening Spyder for the first time, I didn't see the path that I added. I readded and I got the deactivated state correctly set. After that subsecuent Spyder sessions showed the path (not totally sure what happened here 🤔 )

I wonder if this is due to the configuration version bump. I think I've seen similar behavior in other contexts, but didn't pay much attention. I'd have to verify, but this may not be related to this PR.

Also document its most important signal and fix a test that uses that API.

…nit__ This fixes the export button style not being set on Windows

jitseniesen · 2022-11-19T21:48:02Z

So please check if spyder-unittest and spyder-line-profiler keep working with this PR.

Yes, it all works!

Minor wrinkle: I got the error

spyder.api.exceptions.SpyderAPIError: Plugin "pythonpath_manager" not found!

which I fixed with pip install --no-deps -e . in the Spyder directory; hopefully that is correct.

ccordoba12 · 2022-11-19T23:24:31Z

Yes, it all works!

Great! That's really good news @jitseniesen!

Minor wrinkle: I got the error

Right, that's because Spyder is now installed in development mode when run from bootstrap.py, and in that case new entry points for plugins can only detected by reinstalling it. However, that won't be a problem for regular users when 5.4.1 is out.

which I fixed with pip install --no-deps -e . in the Spyder directory; hopefully that is correct.

Yep, that's correct. Another option I use is

pip uninstall spyder
python bootstrap.py

which reinstalls Spyder in development mode too.

I think the last thing to do here is for @dalthviz to review my code, given that @mrclary also said that things are working fine for him.

dalthviz

Thanks @ccordoba12 for all the work here! Checking, the only thing I'm not sure if is expected is the change on the paths placement over sys.path when opening/closing a project and modifing the paths with the manager. When opening or closing a project the paths get added at the end of the IPython Console sys.path but if you change them (remove/disable or add/enable a path) then they appear at the start of sys.path:

Other than that this looks good to me and if the behavior described above is expected then we could merge this 👍

mrclary

Looks good to me. As we discussed before, if there are any additional necessary updates, we can make follow-up PRs for either 5.x or 6.x, as appropriate.

Thanks for all your hard work on this!

ccordoba12 · 2022-11-24T18:11:12Z

Checking, the only thing I'm not sure if is expected is the change on the paths placement over sys.path when opening/closing a project and modifing the paths with the manager. When opening or closing a project the paths get added at the end of the IPython Console sys.path but if you change them (remove/disable or add/enable a path) then they appear at the start of sys.path

Thanks for the extra check @dalthviz! This is not the expected behavior (paths should always be added at the end of sys.path for now), but I can't reproduce it on Linux.

So, I'll check if this is a Windows specific bug on my VM and if that's the case, I'll try to fix it here.

ccordoba12 · 2022-11-27T17:07:11Z

@dalthviz, I can't reproduce the problem you reported on Windows either (I tried to follow the steps you posted as close as possible):

I also tried several other possibilities to reproduce the problem, but paths were always added to the end of sys.path for me.

dalthviz · 2022-11-28T17:18:41Z

Thanks for checking @ccordoba12 , with further testing seems like this happens only when you use a custom interpreter. Could you check setting a custom interpreter and see if with that you can reproduce the issue?

mrclary · 2022-11-28T17:40:23Z

Thanks for checking @ccordoba12 , with further testing seems like this happens only when you use a custom interpreter. Could you check setting a custom interpreter and see if with that you can reproduce the issue?

I've checked on macOS with a custom interpreter and deleting the .spyder-py3-dev configuration directory; I am unable to reproduce the issue. @dalthviz, what happens if you delete your config directory and start fresh?

dalthviz · 2022-11-28T18:31:38Z

Checked using the --safe-mode flag (that should start with a fresh config, right?) but I got the same behavior after opening a project

ccordoba12 · 2022-11-28T23:35:18Z

Could you check setting a custom interpreter and see if with that you can reproduce the issue?

Ok, will do.

mrclary · 2022-11-29T03:55:50Z

Checked using the --safe-mode flag (that should start with a fresh config, right?) but I got the same behavior after opening a project

I think --safe-mode should do the same thing, but not sure. You might try moving .spyder-py3-dev just to see. 🤷🏼‍♂️

ccordoba12 · 2022-11-29T18:04:31Z

Thanks for checking @ccordoba12 , with further testing seems like this happens only when you use a custom interpreter

Sorry @dalthviz, I still can't reproduce your problem after setting a custom interpreter.

In order to move forward with this, I propose that you open a new issue for it so you don't forget to take a look at it when you have time. And then merge this one.

ccordoba12

Some hints for you when merging to master @dalthviz.

spyder/app/mainwindow.py

dalthviz · 2022-11-29T18:18:01Z

@ccordoba12 if you want go ahead and merge this one (since it also requires doing things to master probably is better for you to merge it). Maybe the behavior I'm experiencig is due to my specific preferences (is kind of strange but no idea what is happening there).

Also, as a last suggestion, could be nice to have a test that checks that the paths are being added in the correct position to sys.path in different circumstances (with a project open, with paths being enable and disabled, etc).

mrclary · 2022-11-29T18:24:34Z

Also, as a last suggestion, could be nice to have a test that checks that the paths are being added in the correct position to sys.path in different circumstances (with a project open, with paths being enable and disabled, etc).

I think this will be best implemented on a follow-up PR when we add the feature for path placement, i.e. begin/end of sys.path

ccordoba12 · 2022-11-29T18:28:56Z

@ccordoba12 if you want go ahead and merge this one (since it also requires doing things to master probably is better for you to merge it)

Ok, will do.

Also, as a last suggestion, could be nice to have a test that checks that the paths are being added in the correct position to sys.path in different circumstances (with a project open, with paths being enable and disabled, etc).

I agree with @mrclary because I have several other things to do for 5.4.1 and this has taken a lot of my time (I can add the test in 5.4.2).

What do you say @dalthviz?

dalthviz · 2022-11-29T18:31:06Z

Sounds good to me 👍

ccordoba12 · 2022-11-29T18:44:53Z

Great! I opened issue #20129 so I don't forget to address the test you suggested @dalthviz in 5.4.2.

mrclary · 2022-11-29T18:48:30Z

Also, @dalthviz, you might check spyder-kernels. At present, we have two mechanisms for populating sys.path: first on console creation, in which paths are processed via SPY_PYTHONPATH; second on path change signal, in which spyder-kernels modifies sys.path. I suspect that spyder-kernels is the culprit in the issue that you are seeing.

Fixes #19724

ccordoba12 added 18 commits October 20, 2022 14:29

Path manager: Don't make PYTHONPATH strings translatable

639e0ce

There is no need to have them as such.

Path manager: Add headers for user and project paths

f72b570

This breaks the path list into sections so that it's easier to understand where each path comes from.

LSP: Remove unused method

aae48a2

[WIP] Path manager: Show and handle system PYTHONPATH on it

ce76b3f

This is an intermediate and unfinished commit because after working on this feature I realized that we really need a plugin to implement it in a simple and well structure way.

Add initial structure for a new Pythonpath manager plugin

d525f82

Pythonpath: Make manager dialog show again saved paths

b9fcfe7

Pythonpath: Make it show project path again

3c4515c

Pythonpath: Make it save user paths again

f6969c1

Pythonpath: Connect it to all other plugins that depend on it

123a8bb

Also use spyder_pythonpath option from this plugin instead of the one stored in the main window settings.

Pythonpath: Make PathManager inherit from SpyderWidgetMixin

3bf8280

Pythonpath: Make project path to be in front of the others

6dd87a9

Pythonpath: Add utility functions to check user paths and get system …

b18632c

…Pythonpath

API: Add on_close method to PluginMainContainer

dbfadd3

Pythonpath: Save current system path on its container

a4505e8

Testing: Fix tests for Pythonpath's PathManager widget

26e487f

Pythonpath: Don't include project paths among paths that are exported

57c7ae6

Also fix test related to that functionality

Pythonpath: Remove method in PathManager that is not necessary anymore

1babc37

Pythonpath: Show buttons to the right of the list of paths

b5ac51e

Also use SpyderWidgetMixin API to create them and change their icons

ccordoba12 added this to the v5.4.0 milestone Oct 27, 2022

ccordoba12 requested review from mrclary and dalthviz October 27, 2022 22:58

ccordoba12 self-assigned this Oct 27, 2022

This was referenced Oct 27, 2022

PR: Pass PYTHONPATH to the kernel if not handled by Spyder (IPython console) #19194

Closed

PR: Modernize PYTHONPATH Manager configuration #18778

Closed

mrclary reviewed Oct 28, 2022

View reviewed changes

spyder/config/main.py Outdated Show resolved Hide resolved

spyder/plugins/pythonpath/widgets/pathmanager.py Outdated Show resolved Hide resolved

dalthviz reviewed Oct 28, 2022

View reviewed changes

ccordoba12 added 2 commits October 28, 2022 13:09

Pythonpath: Declare a public API for its plugin

a310e60

Also document its most important signal and fix a test that uses that API.

Pythonpath: Apply buttons style at the beginning of PathManager's __i…

b41d585

…nit__ This fixes the export button style not being set on Windows

ccordoba12 force-pushed the fix-pythonpath branch from 42f8e9a to 5ced99f Compare November 13, 2022 16:23

Merge branch '5.x' into fix-pythonpath

e8db867

dalthviz reviewed Nov 22, 2022

View reviewed changes

mrclary self-requested a review November 23, 2022 21:48

mrclary approved these changes Nov 23, 2022

View reviewed changes

ccordoba12 commented Nov 29, 2022

View reviewed changes

spyder/app/mainwindow.py Show resolved Hide resolved

spyder/app/mainwindow.py Show resolved Hide resolved

spyder/app/mainwindow.py Show resolved Hide resolved

ccordoba12 mentioned this pull request Nov 29, 2022

Check that paths are added in the expected order and to expected position in sys.path by the Pythonpath manager #20129

Closed

ccordoba12 merged commit 623eeca into spyder-ide:5.x Nov 29, 2022

ccordoba12 deleted the fix-pythonpath branch November 29, 2022 18:45

ccordoba12 added a commit that referenced this pull request Nov 29, 2022

Merge from 5.x: PR #19937

2daa2b2

Fixes #19724

ccordoba12 added type:Enhancement component:Pythonpath manager labels Dec 8, 2022

dalthviz mentioned this pull request Mar 3, 2023

Spyder won't launch with update to 5.4.2 #20621

Closed

10 tasks

jitseniesen mentioned this pull request Mar 14, 2023

Ensure compatibility with Spyder 6 spyder-ide/spyder-unittest#195

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PR: Add a new plugin for the Pythonpath manager #19937

PR: Add a new plugin for the Pythonpath manager #19937

ccordoba12 commented Oct 27, 2022 •

edited

Loading

mrclary commented Oct 28, 2022

dalthviz left a comment •

edited

Loading

mrclary commented Oct 28, 2022

jitseniesen commented Nov 19, 2022

ccordoba12 commented Nov 19, 2022 •

edited

Loading

dalthviz left a comment

mrclary left a comment

ccordoba12 commented Nov 24, 2022

ccordoba12 commented Nov 27, 2022

dalthviz commented Nov 28, 2022

mrclary commented Nov 28, 2022

dalthviz commented Nov 28, 2022

ccordoba12 commented Nov 28, 2022

mrclary commented Nov 29, 2022

ccordoba12 commented Nov 29, 2022

ccordoba12 left a comment

dalthviz commented Nov 29, 2022

mrclary commented Nov 29, 2022 •

edited

Loading

ccordoba12 commented Nov 29, 2022

dalthviz commented Nov 29, 2022

ccordoba12 commented Nov 29, 2022

mrclary commented Nov 29, 2022 •

edited

Loading

PR: Add a new plugin for the Pythonpath manager #19937

PR: Add a new plugin for the Pythonpath manager #19937

Conversation

ccordoba12 commented Oct 27, 2022 • edited Loading

Description of Changes

Issue(s) Resolved

Affirmation

mrclary commented Oct 28, 2022

dalthviz left a comment • edited Loading

Choose a reason for hiding this comment

mrclary commented Oct 28, 2022

jitseniesen commented Nov 19, 2022

ccordoba12 commented Nov 19, 2022 • edited Loading

dalthviz left a comment

Choose a reason for hiding this comment

mrclary left a comment

Choose a reason for hiding this comment

ccordoba12 commented Nov 24, 2022

ccordoba12 commented Nov 27, 2022

dalthviz commented Nov 28, 2022

mrclary commented Nov 28, 2022

dalthviz commented Nov 28, 2022

ccordoba12 commented Nov 28, 2022

mrclary commented Nov 29, 2022

ccordoba12 commented Nov 29, 2022

ccordoba12 left a comment

Choose a reason for hiding this comment

dalthviz commented Nov 29, 2022

mrclary commented Nov 29, 2022 • edited Loading

ccordoba12 commented Nov 29, 2022

dalthviz commented Nov 29, 2022

ccordoba12 commented Nov 29, 2022

mrclary commented Nov 29, 2022 • edited Loading

ccordoba12 commented Oct 27, 2022 •

edited

Loading

dalthviz left a comment •

edited

Loading

ccordoba12 commented Nov 19, 2022 •

edited

Loading

mrclary commented Nov 29, 2022 •

edited

Loading

mrclary commented Nov 29, 2022 •

edited

Loading