search

spine-tools / Spine-Toolbox

Spine Toolbox is an open source Python package to manage data, scenarios and workflows for modelling and simulation. You can have your local workflow, but work as a team through version control and SQL databases.
https://www.tools-for-energy-system-modelling.org/
GNU Lesser General Public License v3.0
72 stars 17 forks source link

New project directory structure - [merged] #1165

Closed spine-o-bot closed 3 years ago

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 14, 2019, 17:58

_Merges issue#257_new_project_directorystructure -> dev

Resolves issue #257. Projects are stored into a directory after this. You can still open old .proj files but they will be upgraded to a new style project immediately. There's a new class called ProjectUpgrader that we can hopefully use in the future to upgrade projects automatically when we need to make changes to how projects are saved. There's a new key in all project.json files ('version') that is meant to indicate the project file version.

File paths are now saved to project.json as absolute or relative. If the file is in the project directory, the path is saved as relative. If the file is outside the project directory, the path is saved as absolute. This affects the following:

To test:

  1. Start App
  2. App starts without a project even if you had the Open recent project at start up selected.
  3. Go to File -> Open Project. This opens a new dialog where you can either choose an old .proj file to open or later when you have one a Spine Toolbox project directory
  4. Choose an old .proj file to open. Press ok
  5. This forces you to save the old project using the new style immediately by letting you choose a project Directory
  6. A new directory will be created and all project item data is copied to the new directory. Check that all project item data is copied successfully. If you choose a directory that is already a Spine Toolbox project, it will ask if you want to overwrite the existing.
  7. NOTICE Even though data files have been copied to the new project directory, nothing in the project item Properties has been updated. For example, if you have Data Stores that reference .sqlite files in the old project directory, these will still reference the old directory until you manually update them. Also, e.g. Exporter settings may need some updating.
  8. Check something else. e.g. Tool execution.
  9. You can now change the Project name by just going to Settings and typing a new Project name to the appropriate line edit. Check this too. Known issue: This will make an extra entry in File -> Open recent list.
  10. Work directory is no longer a Project setting so this is not saved to project information json (/.spinetoolbox/project.json). The work directory is saved via QSettings just like any other application global setting. Change this and see if it works.
  11. File -> Save Project As... makes a duplicate of the Project into another directory and opens the new one.
  12. Save project, exit app, and check that the previous project is loaded successfully
  13. Create a new Project. As a new step, you need to provide a directory for the new project.

TODO:

TLDR; Test and give feedback, please.

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 15, 2019, 15:51

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 15, 2019, 15:51

Fixed removing and adding Tool specifications.

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 15, 2019, 18:26

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 15, 2019, 18:30

In the end, I decided not to make any directories like 'my_input_data' since it's likely that users won't dare to use them or they think it's a stupid name or something. We'll add them later automatically if need be.

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 15, 2019, 18:37

added 4 commits

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 19, 2019, 13:45

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 19, 2019, 13:53

File paths are now saved as relative. The paths are handled in app as absolute like before but when the project is saved, all paths are converted relative to project directory. When a project is opened, all paths are converted back to absolute. The converted ones are:

@soininen I don't know if the changes I did to Exporter saving are working though, since it seems that the execution does not really work that well and for some reason it does not remember the selected file name. I may need some help with that one.

spine-o-bot commented 3 years ago

In GitLab by @ererkka on Nov 19, 2019, 13:54

Trying to add a new data store, clicking ok does not do anything. I get this to the console:

Traceback (most recent call last):
  File "C:\Users\ERERKKA\Documents\Python\SpineToolbox\spinetoolbox\widgets\add_project_item_widget.py", line 103, in ok_clicked
    self.call_add_item()
  File "C:\Users\ERERKKA\Documents\Python\SpineToolbox\spinetoolbox\project_items\data_store\widgets\add_data_store_widget.py", line 41, in call_add_item
    self._project.add_project_items("Data Stores", item, set_selected=True)
  File "C:\Users\ERERKKA\Documents\Python\SpineToolbox\spinetoolbox\project.py", line 267, in add_project_items
    item = item_maker(self._toolbox, **item_dict)
  File "C:\Users\ERERKKA\Documents\Python\SpineToolbox\spinetoolbox\project_items\data_store\data_store.py", line 51, in __init__
    self._url = self.parse_url(url)
  File "C:\Users\ERERKKA\Documents\Python\SpineToolbox\spinetoolbox\project_items\data_store\data_store.py", line 76, in parse_url
    if url["database"].lower().endswith(".sqlite"):
KeyError: 'database'

(Works in dev.)

spine-o-bot commented 3 years ago

In GitLab by @soininen on Nov 19, 2019, 15:00

I can confirm there is an issue with Exporter. I'll have a look.

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 19, 2019, 15:33

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @soininen on Nov 20, 2019, 13:18

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @soininen on Nov 20, 2019, 13:19

3c2f73dc should fix the Exporter issue.

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 20, 2019, 14:38

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 20, 2019, 14:38

@soininen thanks for helping. I think the exporter saving/restoring is now halfway there. Below is my Exporter 1 dict in project.json when I saved a project with a Data Store -> Exporter 1 connection. Shouldn't we also make the keys of database_to_file_name_map relative? Is the prefix sqlite:/// really necessary?

        "Exporters": {
            "Exporter 1": {
                "short name": "exporter_1",
                "description": "",
                "x": 357.54319175466,
                "y": -45.86902788957809,
                "database_to_file_name_map": {
                    "sqlite:///C:\\data\\SpineToolboxProjects\\Empty\\.spinetoolbox\\items\\data_store_2\\Data Store 2.sqlite": ".spinetoolbox\\items\\exporter_1\\spinedb"
                },
                "settings_file_names": [
                    ".spinetoolbox\\items\\exporter_1\\export_settings_1.json"
                ]
            }
        }
spine-o-bot commented 3 years ago

In GitLab by @soininen on Nov 20, 2019, 15:44

I would like to keep the URL as-is. My biggest issue is with relative paths referring to databases outside the project directory which will break if the project folder is moved within the file system. I expect most databases not reside in the project directory.

Additionally, we'd need to carefully check if a database URL can be converted to relative path (not possible for network locations). Further, we'd need to store the relative paths as-is as URLs cannot be relative. Checking if the stored value is a URL or a relative path is messy. For relative paths we'd also lose the sqlite:// prefix which is actually necessary to rebuild the URL unless we always assume an sqlite database.

Anyway, I can try to squeeze relative paths in if you so wish. Let me know your thoughts.

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 20, 2019, 15:55

Well, the relative paths were not my idea so It's @ererkka's call to decide what to do with them.

spine-o-bot commented 3 years ago

In GitLab by @ererkka on Nov 21, 2019, 09:07

Original target was that the project would be easily moved in the filesystem or shared with another user. Some data stores are local to the project (intermediate storage etc.), where one generally might want to put the database in the project dir. In this case the url should be relative to the project dir for portability. If the user decides to use an absolute path (network or file path), it’s their responsibility to make sure the location is reachable by the other users.

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 21, 2019, 11:57

I don't know how we could let users decide if they want to save the paths as relative or absolute. I guess it's doable but it would be a nightmare to maintain.

A good option would be to save the file paths in project directory as relative and any others as absolute. This would support moving the project directory anywhere on the file system and both relative and absolute paths should work. If we save file paths outside the project directory as relative, these won't work after the project directory is moved.

But, there's still the same problem with Exporter items. If we go with what I (and Antti) proposed in the paragraph above, the database_to_file_name_map key (the one starting with sqlite:///) should be saved as a relative path. So, I really don't know what to do now...

"Exporters": {
            "Exporter 1": {
                "short name": "exporter_1",
                "description": "",
                "x": 357.54319175466,
                "y": -45.86902788957809,
                "database_to_file_name_map": {
                    "sqlite:///C:\\data\\SpineToolboxProjects\\Empty\\.spinetoolbox\\items\\data_store_2\\Data Store 2.sqlite": ".spinetoolbox\\items\\exporter_1\\spinedb"
                },
                "settings_file_names": [
                    ".spinetoolbox\\items\\exporter_1\\export_settings_1.json"
                ]
            }
        }
spine-o-bot commented 3 years ago

In GitLab by @ererkka on Nov 21, 2019, 12:04

I agree, files in project dir should be saved as relative, other files as absolute.

The SQLite files are also just files. If they reside in the project dir, the paths should be saved relative to the project root. The url can be build from the absolute path to the project dir and the (relative) .sqlite file path.

spine-o-bot commented 3 years ago

In GitLab by @ererkka on Nov 21, 2019, 12:07

It seems that the databases (or Data Stores?) should be identified with something else than the url to avoid using the absolute file path in the url.

spine-o-bot commented 3 years ago

In GitLab by @soininen on Nov 21, 2019, 12:08

Instead of saving just a path string we could have a bit more informative dict in the project file. It could contain information on whether the path is relative and whether it is actually a URL and the URLs scheme. This would make it easier to deal with relative/absolute URLs etc.

spine-o-bot commented 3 years ago

In GitLab by @soininen on Nov 21, 2019, 12:17

I can see if I can implement both relative and absolute paths and support URLs at the same time.

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 21, 2019, 12:17

@soininen do you mean "a bit more informative" dict just for the Exporter items?

spine-o-bot commented 3 years ago

In GitLab by @soininen on Nov 21, 2019, 12:18

I think we should have the same machinery working for all items.

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 21, 2019, 12:26

Remember, that Data Stores already store a url dict that contains a dialect key (e.g. "sqlite") and then the database key that has the relative path to the .sqlite file. I'm not sure if we should touch that, since the url dict is something that SQLAlchemy can read directly (at least that is my understanding)

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 21, 2019, 19:24

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 21, 2019, 19:53

added 30 commits

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @soininen on Nov 22, 2019, 15:22

added 5 commits

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 22, 2019, 19:14

Thanks @soininen, things seem to work quite well now. Have to do some tests and this is done.

I'm just wondering about spreading the responsibilities of ProjectUpgrader all over the place. Are you sure it's a good idea? I'd rather keep it all in one place so that we don't need to maintain the upgrade_from_no_version_to_version_1() method from here to eternity. I was thinking that maybe in six months or so when all project's are in version 1 (at minimum), then we could just drop the support for the old .proj projects all together. This would be really simple if all upgrade related stuff was in ProjectUpgrader class. If we keep it like you have suggested, what do we do when we move from version 1 to version 2? would we need to add upgrade_from_version1_to_version2() methods all over the place again? Or how do you see it?

spine-o-bot commented 3 years ago

In GitLab by @soininen on Nov 25, 2019, 08:36

Initially I did add all the project item upgrading stuff to project_upgrader.py but decided it wasn't a good idea in the view of project items being 'plugins'. In that sense the upgrader should know nothing of individual items and rather let them handle their upgrades themselves.

However, I also though that the upgrader was there to stay. If we are to drop the 'no version -> version 1' upgrade code in, say, 6 months I think it is totally OK to move the project item specific code back to project_upgrader.py.

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 26, 2019, 18:00

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 26, 2019, 18:25

added 27 commits

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 27, 2019, 14:43

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 27, 2019, 18:40

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 11:36

added 9 commits

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 13:06

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 13:55

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 16:45

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 16:49

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 19:17

added 1 commit

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 19:18

added 4 commits

Compare with previous version

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 19:20

marked the task Do something about absolute paths in project.json files as completed

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 19:25

unmarked as a Work In Progress

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 19:25

changed the description

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 19:29

changed the description

spine-o-bot commented 3 years ago

In GitLab by @PekkaSavolainen on Nov 28, 2019, 19:33

This is now ready to be reconsidered for merging. You still interested @ererkka, @soininen? @manuelma I would be really interested to hear what you think as well.

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Nov 28, 2019, 19:35

Tomorrow I'll review it.

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Nov 28, 2019, 21:50

The order of the arguments in the docstring is incorrect.

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Nov 28, 2019, 21:58

ToolInstance or ToolSpecification?

spine-o-bot commented 3 years ago

In GitLab by @manuelma on Nov 28, 2019, 21:59

Again, I think it returns a ToolSpecification