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

Change on Python Basic Console not transferred to tools #2446

Closed
dlr-cjs opened this issue Dec 1, 2023 · 14 comments
Closed

Change on Python Basic Console not transferred to tools #2446

dlr-cjs opened this issue Dec 1, 2023 · 14 comments
Assignees

Comments

@dlr-cjs
Copy link

dlr-cjs commented Dec 1, 2023

Description

If I change the Basic Console under File -> Settings -> Tools -> Python, these changes do not take effect in tools of the project workflow. Instead, I would have to open each Python-based tool in the *Tool specification editor", change the Tool type to, e.g., Julia, then change back to Python - only now will the Interpreter be updated to the one specified in the settings.

To reproduce

  • open any existing project in spinetoolbox that has a python tool using "Basic console"
  • change the Basic Console Python interpreter at under File -> Settings -> Tools -> Python
  • open Tool specification editor for any python tool in your workflow
  • see that the Basic console interpreter was not updated

Setup

Python: 3.9
spinetoolbox-dev: 1.0a1
spinetoolbox: 0.8.0.dev9+g24d07cef.d20231201

@PekkaSavolainen
Copy link
Member

This is not a bug but a feature. The Python interpreter (and Julia Executable) settings in File->Settings->Tools are just the default settings for new Tool specifications. This is mentioned in the User Guide and in the File->Settings->Tools tooltips.

However, we could add a button into the Tool Specification Editor that automatically updates the execution settings into the default settings defined in File->Settings->Tools.

@dlr-cjs
Copy link
Author

dlr-cjs commented Dec 1, 2023

If this is a feature, it makes sharing workflows unpleasant, since each user needs to update a workflow (possible large workflow comprising dozens of tools) manually. I suggest a button on the workflow level allowing to "update basic console" in all comprising tools.

To be more precise what my issue is:
I am using spinetoolbox in TradeRES to share my workflow with others. I require a different environment for my Python tool "AMIRIS", since we have a higher minimum requirement for protobuf. Thus, I need the users to change to another environment for the Python tools in my workflow (luckily its only three tools).

Currently, users need to follow these instructions to accomplish this... which is rather annoying.

Thus, the "update console" process should be as easy as possible.

@PekkaSavolainen
Copy link
Member

I suggest a button on the workflow level allowing to "update basic console" in all comprising tools.

This is a good idea.
,
But I'm not sure why you are instructing user's to edit the default python interpreter and then in each Tool spec you switch to Julia and then back to Python? I guess you want to avoid navigating to the same Python interpreter file multiple times, which can be pretty tedious.

There is however a little bit simpler option available. You could try running the Tool Specs with the Jupyter Console option. There is a section in the user guide on how to do this. https://spine-toolbox.readthedocs.io/en/master/setting_up.html#conda

When you have created the AMIRIS Conda environment, you could simply instruct users to:

  1. Make sure that the Select Miniconda executable is set in File->Settings->Tools
  2. Double click on a Tool in Design View
  3. Select Jupyter Console radio button
  4. Select the AMIRIS envronment from the dropdown 'kernel' menu. The name in the list is not AMIRIS but something like conda-env-.conda-AMIRIS-py
  5. Repeat 2&3&4 for each Tool in project

But this is not ideal and it would be a lot simpler with your suggestion.

PekkaSavolainen added a commit that referenced this issue Dec 13, 2023
…gs for Julia and Python Tools in the current project

Re #2446
PekkaSavolainen added a commit to spine-tools/spine-items that referenced this issue Dec 13, 2023
PekkaSavolainen added a commit to spine-tools/spine-items that referenced this issue Jan 17, 2024
…l Specifications (#179)

In addition, add 'restore defaults' button to Tool Spec Editor for Python and Julia Tool specs

Re spine-tools/Spine-Toolbox#2446
PekkaSavolainen added a commit that referenced this issue Jan 17, 2024
…ython Tools (#2461)

- Add buttons to Settings->Tools widget to set default execution settings for Julia and Python Tools in the current project
- Updated user guide and changelog

Re #2446
@PekkaSavolainen
Copy link
Member

I added two buttons to Settings->Tools widget. The upper one sets all Julia Tools in the current project to use the execution settings that are defined on the same page (Basic/jupyter Console, Julia executable path, Julia env, and kernel. The lower button does the same for all Python tools in the current project.

issue_2446_new_buttons_in_settings

There's also a new button in the Tool Spec Editor, which does the same for individual Tool Specs. The button is disabled (grayed out) if the current selections match the default settings defined in Settings->Tools.

issue_2446_new_button_in_tool_spec_editor

This is in 0.8-dev branch and it will trickle down to master branch eventually.

@dlr-cjs
Copy link
Author

dlr-cjs commented Jan 17, 2024

Very cool! Thank you, that is super useful for our case.

@soininen
Copy link
Contributor

After being hit with the same issue, I think the behavior and the "broadcast" buttons are confusing. The fact that the Python interpreter in Settings only affects new Tool specifications is not really what I would expect and the "broadcast" button is difficult to discover. I suggest the following behavior:

  • If Tool has an interpreter set explicitly, it will be used no matter what.
  • Otherwise, if Settings provides an interpreter, Tool uses it.
  • Otherwise, use the interpreter provided by resolve_*() functions.

This way we can drop the "broadcast" buttons in Settings and the Restore defaults button in Tool specification editor as clearing the Interpreter field will do the same.

@soininen soininen reopened this Feb 15, 2024
@PekkaSavolainen
Copy link
Member

It's not that simple. Consider, what happens in the following case.

  1. I open a project with Tool A using Julia Spec_A with no executable set
  2. I go to Settings, change the Julia to <somejulia/bin/julia.exe>, click Ok
  3. Spec_A now uses <somejulia/bin/julia.exe>
  4. Then I open another project, which has a Tool B with Julia Spec_B with no executable set
  5. Which executable does Spec_B use now?

There are two options:

  1. If it uses 'no executable' (i.e. the one in path), then there's a conflict with the one in Settings->Tools. If a user now executes Tool B, then opens the Settings window and clicks Ok, then executes Tool B again, the first and second executions will happen on a different Julia. Not very user friendly IMO.
  2. If it uses <somejulia/bin/julia.exe>, then toolbox has overwritten the Spec_B executable. This means that Tool Specs cannot be saved with the 'no interpreter set' option anymore, because this will always be overwritten if Settings provides an interpreter.

TLDR:
The 'broadcast' buttons are there because the options in Settings->Tools are global settings. If we want to make them as project level settings then that brings another set of problems with consistency and discoverability.

@soininen
Copy link
Contributor

soininen commented Feb 15, 2024

Which executable does Spec_B use now?

B would use the Julia in Settings, i.e. same Julia a A <somejulia/bin/julia.exe>

If it uses <somejulia/bin/julia.exe>, then toolbox has overwritten the Spec_B executable. This means that Tool Specs cannot be saved with the 'no interpreter set' option anymore, because this will always be overwritten if Settings provides an interpreter.

But that is what global settings are supposed to do. If you want to lock B's interpreter, then you set in the specification. You use 'no interpreter set' to say "I am happy with whatever is in Settings."

@PekkaSavolainen
Copy link
Member

I guess that makes sense. It's possible that I was overthinking the issue.

@soininen
Copy link
Contributor

There could be a way to set the interpreter/executable/shell for multiple specifications at once but I think it should be separate from the Settings window.

@dlr-cjs
Copy link
Author

dlr-cjs commented Feb 16, 2024

I would argue differently. Especially for Python, a each project is likely to have its own interpreter path. Therefore, I think if you really use spine-toolbox in mutliple projects, these python and julia defaults should be stored and controllable on a "per project" basis. I, for myelf only have one toolbox project,so I don't mind personally, but other users might benefit from such a "hierarchy":

general default <-> project default <-> tool-specific settings

@soininen
Copy link
Contributor

Especially for Python, a each project is likely to have its own interpreter path.

Agreed. We should have Project-specific settings, too.

@soininen soininen self-assigned this Feb 16, 2024
@jkiviluo
Copy link
Member

In a perfect world, each tool would know what they need (requirements file). We could then check if the environment satisfies the requirements - if not, we could offer to setup a new clean environment and install requirements. Or optionally upgrade current environment, but there one needs to be careful, because that environment might be used for other things too.

We also want all of this hard installation part to be on the shoulders of workflow developers - not workflow users. So, this relates: #2554

As you can see, I'm bit vary of global and project defaults - I don't want to say they are all bad, but I'm afraid they may lead to trouble down the road (e.g. you change your global environment and things break).

soininen added a commit to spine-tools/spine-engine that referenced this issue Feb 19, 2024
resolve_python_interpreter() has now been refactored into two functions:
resolve_current_python_interpreter() returns the current Python executable
while resolve_python_interpreter() returns returns the interpreter set in
application settings.

Re spine-tools/Spine-Toolbox#2446
soininen added a commit to spine-tools/spine-engine that referenced this issue Feb 19, 2024
resolve_julia_executable() has now been refactored into two functions:
resolve_default_julia_executable() returns the Julia executable from PATH
and resolve_julia_executable() returns returns the executable set in
application settings.

Re spine-tools/Spine-Toolbox#2446
soininen added a commit that referenced this issue Feb 19, 2024
Tool specifications now always use the global Python interpreter
from settings so we do not need to explicitly broadcast it to
specifications.

Re #2446
soininen added a commit that referenced this issue Feb 19, 2024
Tool specifications now always use the global Julia executable
from settings so we do not need to explicitly broadcast it to
specifications.

Re #2446
soininen added a commit to spine-tools/spine-items that referenced this issue Feb 19, 2024
We now always use the global interpreter/executable from application settings
if none is excplicitly set in Tool Specification.

Re spine-tools/Spine-Toolbox#2446
@soininen
Copy link
Contributor

I am happy with how the interpreter/executable global settings work.

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

No branches or pull requests

4 participants