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 Spyder PYTHONPATH to PluginClient #4263

Merged
merged 5 commits into from
Mar 21, 2017

Conversation

rlaverde
Copy link
Member

@rlaverde rlaverde commented Mar 15, 2017

Fixes #4141


I think this is ready, but this is causing plugins to be restarted at startup, (normally 1 time, and 2 times when there is a project open)

It's that ok? or how It could be avoided?

@rlaverde rlaverde added this to the v3.1.4 milestone Mar 15, 2017
@rlaverde rlaverde self-assigned this Mar 15, 2017
@ccordoba12
Copy link
Member

@rlaverde, please add a test for this. From now on I want to see tests for every new features we add.

@@ -1462,6 +1462,8 @@ def set_current_filename(self, filename, editorwindow=None):
def set_path(self):
for finfo in self.editorstacks[0].data:
finfo.path = self.main.get_spyder_pythonpath()
if self.introspector:
self.introspector.change_spyder_path(self.main.get_spyder_pythonpath())
Copy link
Member

Choose a reason for hiding this comment

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

It looks wider then 79 columns.

@@ -41,7 +41,7 @@ class AsyncClient(QObject):
received = Signal(object)

def __init__(self, target, executable=None, name=None,
extra_args=None, libs=None, cwd=None, env=None):
extra_args=None, libs=None, cwd=None, env=None, extra_path=None):
Copy link
Member

Choose a reason for hiding this comment

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

Move extra_paths=None to a new line below this one

@@ -86,6 +89,11 @@ def run(self):
python_path = osp.pathsep.join([python_path, path])
except ImportError:
pass
if self.extra_path:
try:
python_path = osp.pathsep.join([python_path] + self.extra_path)
Copy link
Member

Choose a reason for hiding this comment

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

Looks wider than 79

@ccordoba12
Copy link
Member

It's that ok? or how It could be avoided?

This happens because Spyder's PYTHONPATH is updated at startup and when changing projects, so i think you should try to restart plugins exactly when it's needed.

@rlaverde rlaverde force-pushed the fix/python-path-jedi branch 2 times, most recently from 9663163 to e90fb04 Compare March 17, 2017 19:09
- Avoid one innecessary restart at startup,
- Also change the name to extra_path to make it more generic.
@rlaverde rlaverde force-pushed the fix/python-path-jedi branch from e90fb04 to 0a317fd Compare March 17, 2017 19:12
@rlaverde rlaverde force-pushed the fix/python-path-jedi branch from 0a317fd to f943450 Compare March 17, 2017 19:14
@ccordoba12
Copy link
Member

@rlaverde, your test is failing on Windows. Please skip it there.

@rlaverde
Copy link
Member Author

@ccordoba12 I forget that windows use a different path separator, I added a commit to fix that test

@@ -77,7 +81,7 @@ def run(self):
# Set up environment variables.
processEnvironment = QProcessEnvironment()
env = self.process.systemEnvironment()
if (self.env and 'PYTHONPATH' not in self.env) or DEV:
if (self.env and 'PYTHONPATH' not in self.env) or self.env is None:
Copy link
Member

Choose a reason for hiding this comment

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

Please restore DEV here. I think it's needed to make completions work when Spyder is started with bootstrap.py

Copy link
Member Author

Choose a reason for hiding this comment

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

hmm no, I tested and this condition evaluate to True when using ./bootstrap.py

I changed because previously it only evaluates to True when DEV was True (i. e. libraries, and spyder path only will be added in dev mode)

Copy link
Member

Choose a reason for hiding this comment

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

Ok, thanks for the clarification!

@ccordoba12
Copy link
Member

ccordoba12 commented Mar 21, 2017

@rlaverde, how many times are our completion plugins are restarted when switching projects? Two as before, or just one?

@rlaverde
Copy link
Member Author

how many times are our completion plugins are restarted when switching projects? Two as before, or just one?

  • One restart when switching projects
  • No restart at startup
  • One restart at startup with a project open (load and then restart)

@ccordoba12
Copy link
Member

The number of restarts are exactly the ones needed. Thanks for improving that!

@ccordoba12
Copy link
Member

I restarted Circle again to make coveralls to pass before merging.

@ccordoba12 ccordoba12 changed the title PR: Add spyder path to PluginClient PR: Add Spyder PYTHONPATH to PluginClient Mar 21, 2017
@ccordoba12 ccordoba12 merged commit c0f5423 into spyder-ide:3.1.x Mar 21, 2017
ccordoba12 added a commit that referenced this pull request Mar 21, 2017
ccordoba12 added a commit that referenced this pull request Mar 21, 2017
@rlaverde rlaverde deleted the fix/python-path-jedi branch March 22, 2017 14:48
rlaverde added a commit to rlaverde/spyder that referenced this pull request Aug 14, 2017
rlaverde added a commit to rlaverde/spyder that referenced this pull request Aug 15, 2017
rlaverde added a commit to rlaverde/spyder that referenced this pull request Aug 25, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants