-
-
Notifications
You must be signed in to change notification settings - Fork 34
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
Use Spyder's new PythonpathManager plugin #197
Conversation
This is necessary to support Spyder 6
Specifically, if Spyder is installed from Git, then don't try to also install it with conda.
The plugin currently depends on Spyder 6 which is not yet released.
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.
Thanks @jitseniesen for your work on this!
Co-authored-by: Carlos Cordoba <ccordoba12@gmail.com>
Codecov Report
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files
|
Do not import the fixture from Spyder because tests (and thus fixtures) are not distributed with Spyder.
This seems to cause a segmentation fault when pytest exits, for reasons that are not clear. Perhaps the code should be removed from Spyder.
This is no longer necessary now that Spyder PR #20789 is merged.
@ccordoba12 Thanks for resolving the issue in Spyder so quickly. I changed the test fixture to reflect your changes and I think it is ready now. Let me know whether you want to do another review or want me to merge. |
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.
I left a simple question, the rest looks good to me @jitseniesen.
And great work with the integration tests! They are really cool.
plugin.handle_project_change) | ||
plugin.main.projects.sig_project_closed.connect.assert_called_with( | ||
plugin.handle_project_change) | ||
projects._create_project(project_dir) |
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.
Should we make _create_project
a public method in the Projects plugin API? It looks like the appropriate method to call to easily create projects.
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.
@ccordoba12 Yes, it makes sense to make _create_project
public. This does commit us to some API stability which may hinder development of projects in the near future; you should have a decent guess whether this is likely to be a problem. The name is also quite similar to the create_new_project
method but I don't have a solution for that.
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.
Ok, thanks for the feedback.
This PR allows the plugin to run with Spyder 6, so the PR also bumps the plugin version and the dependency to specify that Spyder 6 is installed with the plugin.
Fixes #195
Fixes #167