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

Allows to have main program file in a directory. #238

Merged
merged 12 commits into from
Dec 20, 2024

Conversation

jkiviluo
Copy link
Member

@jkiviluo jkiviluo commented Dec 1, 2024

Fixes spine-tools/Spine-Toolbox#3020

Checklist before merging

  • Documentation (also in Toolbox repo) is up-to-date
  • Release notes in Toolbox repo have been updated
  • Unit tests have been added/updated accordingly
  • Code has been formatted by black & isort
  • Unit tests pass

@jkiviluo jkiviluo requested a review from ptsavol December 1, 2024 21:41
Copy link

codecov bot commented Dec 1, 2024

Codecov Report

Attention: Patch coverage is 70.21277% with 56 lines in your changes missing coverage. Please review.

Project coverage is 63.03%. Comparing base (c266671) to head (98640fd).
Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...s/tool/widgets/tool_specification_editor_window.py 65.85% 19 Missing and 9 partials ⚠️
spine_items/tool/tool.py 55.26% 14 Missing and 3 partials ⚠️
spine_items/commands.py 26.66% 11 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #238      +/-   ##
==========================================
- Coverage   63.08%   63.03%   -0.06%     
==========================================
  Files         168      168              
  Lines       17838    17959     +121     
  Branches     2051     2066      +15     
==========================================
+ Hits        11253    11320      +67     
- Misses       5995     6038      +43     
- Partials      590      601      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ptsavol
Copy link
Member

ptsavol commented Dec 2, 2024

I need a bit of guidance on how to test this. I think I need to edit the tool spec manually somehow but I'm not sure how. And this only works in Python basic console and when executing in 'source directory', right?

@jkiviluo
Copy link
Member Author

jkiviluo commented Dec 2, 2024

Let's say your main program is scripts/run.py.

  • Modify the 'includes' in the tool_specification.json by adding that 'scripts/' in front of filename (if you already have 'run.py' as the main program).
  • Then you also need to modify 'includes_main_path' by taking out '/scripts' from there.

Just noting that 'includes_main_path' is using a relative address starting from the tool specification location, which is not what we really want - the relative location should start from 'root_directory' (or 'source code root directory' to be more descriptive), but this quick fix does not address that yet. So, we should, later, replace 'includes_main_path' with 'source_code_root_directory'. I actually started doing that, but then I realized that 'source_code_root_directory' should be tool property rather than tool specification property (since it can and will differ between computers using the same tool spec - and even within computer if one has multiple versions of the same tool).

@ptsavol
Copy link
Member

ptsavol commented Dec 3, 2024

Let's say your main program is scripts/run.py.

  • Modify the 'includes' in the tool_specification.json by adding that 'scripts/' in front of filename (if you already have 'run.py' as the main program).
  • Then you also need to modify 'includes_main_path' by taking out '/scripts' from there.

Yes, this part works but what if your program needs some data files?

Let's say the root directory of your program is /testprog/ and your main would be in

/testprog/scripts/run.py

then you have a data file

/tesprog/data/data.csv

I don't think this works. Do we need to worry about a situation like this?

@ptsavol
Copy link
Member

ptsavol commented Dec 11, 2024

Hey @jkiviluo, can you please test this? It should now be possible to make a Tool Spec which has the main program file in a directory, directly in the app, without doing any manual edits to Tool Spec .json files.

to test:

  • Get fix_main_program_in_folder branches from BOTH SPINE-ITEMS and SPINETOOLBOX
  • Open Toolbox and appropriate project
  • Select a Tool which has a Tool Spec
  • Set the Root directory in the Tool Properties
  • Double-click the Tool and edit the main program file and additional program files to your liking -> save -> execute item

What do you think?

@jkiviluo
Copy link
Member Author

jkiviluo commented Dec 12, 2024

The root directory is not saved. When I set it up at first, the tool worked. But when I close project and open again, the root directory is gone (and it tries to execute in the sub-directory where the main program file is). And now I can't get it to work at all.

Minor things:

  • In the text box now says: 'Root directory...'. Change that to 'Root directory path.. (defaults to project path)'. And then add label before the box: 'Source code root directory:'
  • When main program file is in a directory, the tool spec editor doesn't show the relative path/filename (it only shows the filename of the main program). Tooltip is correctly showing the full path (nice that tooltip does that).
  • It seems to be easy to mess up the 'additional program files'. Try adding a directory and then adding a separate file from the root directory. You get a directory with ... and things are inside that for some reason. (This was probably an issue before this update).
spine_engine               0.26.0.dev1+g408e5cb  C:\sources\spine-toolbox\src\spine-engine
spine_items                0.24.0.dev14+g3cd01c1 C:\sources\spine-toolbox\src\spine-items
spinedb_api                0.33.0.dev5+g5d71be3  C:\sources\spine-toolbox\src\spinedb-api
spinetoolbox               0.10.0.dev9+g206f4592 C:\sources\spine-toolbox
SQLAlchemy                 1.3.24
stack-data                 0.6.3
tableschema                1.21.0
termcolor                  2.5.0
tornado                    6.4.1
traitlets                  5.14.3
trove-classifiers          2024.10.21.16
typing_extensions          4.12.2
tzdata                     2024.2
unicodecsv                 0.14.1
urllib3                    2.2.3
watchdog                   6.0.0
wcwidth                    0.2.13
wget                       3.2
xlrd                       2.0.1

(flextool) C:\sources\spine-toolbox>python spinetoolbox.py

- Fix saving root directory into local project settings
- Edit root directory lineEdit placeholder text
- Add label above root directory lineEdit
- Browse root directory button now opens the file explorer in the project dir by default
- Enable Clear buttons in both lineEdits in the Tool Properties
@ptsavol
Copy link
Member

ptsavol commented Dec 12, 2024

The root directory is not saved. When I set it up at first, the tool worked. But when I close project and open again, the root directory is gone (and it tries to execute in the sub-directory where the main program file is). And now I can't get it to work at all.

Minor things:

  • In the text box now says: 'Root directory...'. Change that to 'Root directory path.. (defaults to project path)'. And then add label before the box: 'Source code root directory:'

Saving the root directory and the first bullet point has been fixed in 02e3b80.

@ptsavol
Copy link
Member

ptsavol commented Dec 12, 2024

  • When main program file is in a directory, the tool spec editor doesn't show the relative path/filename (it only shows the filename of the main program). Tooltip is correctly showing the full path (nice that tooltip does that).

I do see the relativepath/filename in the tool spec editor, so I'm not sure how to recreate this. Can you try and remove it and add it again (in the tool spec editor) and make sure that the Root directory path in the Tool properties is correct.

image

@ptsavol
Copy link
Member

ptsavol commented Dec 12, 2024

  • It seems to be easy to mess up the 'additional program files'. Try adding a directory and then adding a separate file from the root directory. You get a directory with ... and things are inside that for some reason. (This was probably an issue before this update).

Sorry, I cannot reproduce this either. Can you send me the specification (json) file? Or can you make a new Tool and a Tool Spec for that item from scratch and try again?

@ptsavol
Copy link
Member

ptsavol commented Dec 12, 2024

I forgot to push a commit into SpineToolbox repo fix_main_program_in_folder branch earlier. It's now there. Make sure to pull that branch as well before testing.

@jkiviluo
Copy link
Member Author

I haven't done extensive testing, but I haven't encountered problems while using this now branch now for a while.

@ptsavol
Copy link
Member

ptsavol commented Dec 16, 2024

Is this feature needed for Gams and Julia programs as well? How about generic executables?

@jkiviluo
Copy link
Member Author

Yes, all of them can have that. (In Erkka's old issue it was actually about GAMS)

@jkiviluo
Copy link
Member Author

Traceback (after a crash) on Ubuntu. I opened and closed tool specification. Then, when trying to re-open it, it crashed.

Traceback (most recent call last):
File "/home/prokjt/sources/Spine-Toolbox/spinetoolbox/widgets/custom_qgraphicsscene.py", line 236, in event
return super().event(event)
File "/home/prokjt/python-envs/spi/src/spine-items/spine_items/tool/tool_icon.py", line 83, in mouseDoubleClickEvent
item.show_specification_window()
File "/home/prokjt/python-envs/spi/src/spine-items/spine_items/tool/tool.py", line 256, in show_specification_window
self._toolbox.show_specification_form(self.item_type(), self.specification(), self)
File "/home/prokjt/sources/Spine-Toolbox/spinetoolbox/ui_main.py", line 1588, in show_specification_form
multi_tab_editor.add_new_tab(specification, item, **kwargs)
File "/home/prokjt/sources/Spine-Toolbox/spinetoolbox/widgets/multi_tab_window.py", line 132, in add_new_tab
tab = self._make_new_tab(*args, **kwargs)
File "/home/prokjt/sources/Spine-Toolbox/spinetoolbox/widgets/multi_tab_spec_editor.py", line 34, in _make_new_tab
tab = self._toolbox.item_factories[self.item_type].make_specification_editor(self._toolbox, *args, **kwargs)
File "/home/prokjt/python-envs/spi/src/spine-items/spine_items/tool/tool_factory.py", line 62, in make_specification_editor
return ToolSpecificationEditorWindow(toolbox, specification, item, **kwargs)
File "/home/prokjt/python-envs/spi/src/spine-items/spine_items/tool/widgets/tool_specification_editor_window.py", line 122, in init
self._set_main_program_file(os.path.abspath(os.path.join(self.item.root_dir, main_program_file)))
File "/usr/lib/python3.10/posixpath.py", line 76, in join
a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not NoneType
Segmentation fault (core dumped)

- Serialize & deserialize root directory path when saving & loading a project
- Fix a possible crash
- Make start directories in file browsers more convenient
@ptsavol
Copy link
Member

ptsavol commented Dec 19, 2024

...
TypeError: expected str, bytes or os.PathLike object, not NoneType
Segmentation fault (core dumped)

I'm now catching this TypeError but the reason for the crash is most likely somewhere else.

@ptsavol
Copy link
Member

ptsavol commented Dec 19, 2024

I think this is ready. This new feature should now work with all Tool Spec types (Python, Julia, Gams, Executables), in both Basic and Jupyter Consoles and in both source directory and work directory modes. Should I merge or do you (@jkiviluo) still know of any problems here?

@ptsavol ptsavol marked this pull request as ready for review December 19, 2024 14:31
@ptsavol ptsavol merged commit bfcb7ec into master Dec 20, 2024
10 checks passed
@ptsavol ptsavol deleted the fix_main_program_in_folder branch December 20, 2024 10:31
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.

Not possible to execute main programs from a folder
2 participants