PR: Unify access to current active interpreter

[maurerle]

until now, custom_interpreter is always aligned with executable.
This is fixed here - so that executable always holds the current active used python interpreter, while the custom_interpreter is used to store the last value of the preferences dialog.

I saw, that the custom_interpreter/executable does not behave very good when looking into #1884 which is fixed by this PR.

Another possibility would be to remove the custom_interpreter variable at all.
If custom is set, the custom_interpreter is found in executable, else it is not of interest it is used to store the value of the dialog.
Old values are stored in the custom_interpreters_list, so they are unaffected.
The only actual usage of the custom_interpreter value exists in get_main_interpreter, which is never called too (but is a public API).

If would actually suggest to change get_main_interpreter code to

def get_main_interpreter(self):
    return self.get_conf('executable', get_python_executable())

to unify the access to the current interpreter, which makes it easier for other plugins to get the executable to.

What do you think about that? The current situation seems to be improvable wrong and it is not possible to see the custom_interpreter value in this variable.

Description of Changes

• Wrote at least one-line docstrings (for any new functions)
• Added unit test(s) covering the changes (if testable)

• Included a screenshot or animation (if affecting the UI, see Licecap)

Issue(s) Resolved

Fixes #1884

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: Florian Maurer

[ccordoba12]

I don't think this is correct. I mean, custom_interpreter should be empty until the user selects one.

@dalthviz, what do you think?

[dalthviz]

Following up #17789 (comment) I would say that we should use the get_interpreter method instead of changing the logic for the custom_interpreter option/config

Edit: Actually, the comment related with this is #17789 (comment) . Basically it initializes empty since no custom interpreter is initially selected (it should be empty as @ccordoba12 says I think)

[pep8speaks]

Hello @maurerle! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

• In the file spyder/plugins/maininterpreter/container.py:

Line 63:80: E501 line too long (81 > 79 characters)

Comment last updated at 2022-05-18 17:42:48 UTC

[maurerle]

As mentioned before:

• custom_interpreter is a variable to store the last custom_interpreter for the preferences dialog - it should not be used by other plugins to receive the current interpreter
• executable always contains the current executable and is already checked when set, so additional checks are not needed

The Variable flow is like this:

1. All listeners are executed, checking the availability of the last set executable
2. Opening Preferences - current value gets added to list of executables (plugin._add_to_custom_interpreters(executable))
3. When applying a custom interpreter, the on_conf_change gets triggered for default/custom/custom_interpreter changes, updating the current executable if it needs an update:

if executable != self.get_conf('executable'):
            self.set_conf('executable', executable)

4. the updated executable triggers the on_conf_change handler, which updates the status_widget and emits sig_interpreter_changed for other plugins

I think this is a clean solution and unifies the different mitigation approaches

[ccordoba12]

Hey @maurerle, thanks a lot for this! I thought about it a lot during the weekend and regard it now as a great improvement.

I left a small review for you. After you address it, I can give you a hand to fix our tests.

[maurerle]

Thank you Carlos! I am very happy to contribute it :)
I fixed the discussions so far.

I ran all the tests locally (very impressive suite! Very great!) but did not find a related issue.

The CI tells me that the test in main_widget.py fails because the main_interpreter section does not exist (probably in the mock or the testcase?
You can probably tell best how I can require it in the test?

[ccordoba12]

@maurerle, thanks for addressing my previous suggestions. Second review now, much simpler than the first one.

[ccordoba12]

You can probably tell best how I can require it in the test?

So, we have two kinds of tests: automatic tests based on Pytest and Pytest-Qt, and tests at the end of many files to quickly see if the widget in the file is displayed correctly.

The one that was failing with your changes was a file test and it happened because it doesn't assume that our config options are populated. And the fix was simple: trying to get the executable option with a default value before profiling a test script (as you can see in commit da69a18).

[ccordoba12]

Thanks @maurerle for your contribution!! This is a great improvement!