Explicitly delete database parameters on database delete

[marc-vdm]

fix #823, #756 and #614 Adds additional signal to trigger explicit delete of parameters that belong to (activities in) a database. Previous parameter delete was unreliable and gave no errors when it failed, but broke all parameters until offending parameters were deleted manually.

This PR needs testing by someone who understands parameters very well.

[coveralls]

Coverage increased (+0.06%) to 53.953% when pulling 09a7363a08b125c71a2407edf46a895427b092a9 on marc-vdm:parameter_delete_issue into 03d46b710cb03d260ac2b82b07c3a57b1d343ea1 on LCA-ActivityBrowser:master.

[marc-vdm]

@MaximeAgez, you seem to be an active user of AB -and specifically parameters- and often help out with issues, we really appreciate this support on AB! I've had the above PR for some time to fix issues related to deleting parameters, but we need someone actively using parameters to properly test it.

The PR should resolve the issue of improperly deleting parameters, thus breaking their use in a project. However, I have always had a hard time replicating these kinds of issues and am not an active parameter user myself. As you seem much more experienced with parametes, would you be willing to help test this code to see if it works as intended? Of course, if you wouldn't have the time or interest, that's no problem, we'll find another way then. If you would be interested, please let me know and we can coordinate further.

Best,

Marc

[MaximeAgez]

I would be interested in testing the PR, but I have little experience with unit test and such. I don't know if that's an issue or not.

[marc-vdm]

That's great to hear :).

As you're more experienced with these bugs, you may be able to more easily replicate them. The idea is just that you try out the fix (if you don't know how I can help you set it up) and see if it resolves the issues of #823, #756 and #614. Once you've been able to test things (set up a situation where #823, #756 and/or #614 occur and then where these fixes make the problem go away), if you could let me know your findings, that would be perfect! It's probably safest to test this in a project that is not mission critical, just to be sure.

What I believe the main issue is: I think the main problem with these popups occurs when someone deletes a database with parameters, that the parameters are not properly deleted, which then causes these problems. This PR should explicitly delete the problematic parameters when a database is deleted.

As far as I could find, this should work, but again, I had a very hard time actually replicating the issue, so I'm not 100% sure.

No need to worry about setting up tests for our automated testing etc. That will come later. Also no need to worry about being responsible, I wrote the code, so I should be responsible if it breaks things 😅 and no need to hurry, this has been open for a while, and this is not your project, so that you're even helping is already great!

If you have any questions or something, just let me know!

[MaximeAgez]

@marc-vdm how do I need to setup AB to be able to have it running on the specific PR? Right now I forked the project for the specific PR but I have no idea how to open AB from the AB folders/files.

[marc-vdm]

With a commandline you can move to the folder you downloaded the fork to: e.g. for me C:\Users\*\PycharmProjects\activity-browser (cd to change directory in the windows/linux/mac command line, dir to see which files/folders there are in a folder in windows, ls on linux/mac)

That folder should have a file run-activity-browser.py

then simply activate your AB-dev environment with conda activate ab_dev and then python run-activity-browser.py, this should start AB like you're used to.

[bsteubing]

@marc-vdm and @MaximeAgez thanks for working on this! I was about to accept the PR, but then realized you are still testing it. Please let me know if everything works as intended then I will accept the PR.

[MaximeAgez]

(Sorry it's my first review so I don't know what I'm supposed to report or not.)

I think there is still an issue.

To reproduce my test: I created a new database and a single activity in there. For this activity I created an activity parameter.

Now in the parameters.db there are multiple entries being created:

• the activity parameter itself is created in the activity parameter table, with the name, group, value, code, etc.
• the activity parameter group is create in the group_table table
• a dependency between activity parameter groups is also created in groupdependency table. To have a dependency, I believe the parametrized exchange (the formula) needs to include the activity parameter and either another parameter (either database or project)
• Finally the parametrized exchange in which the activity parameter is called is create in the parametrizedexchange table

In the current AB version: If you delete the activity where the activity parameter was created, everything is deleted smoothly: the activity parameter metadata is gone, the group_table entry is gone, the groupdependency is gone and the parametrizedexchange is gone. No issue there. If instead you delete the database itself, most of the entries are gone except the group_table and the groupdependency entries for some reason. This creates a bug when you recreate the database with the same name and an activity with the exact same name, because a new group_table entry is created and only one can exist. When would that occur? Typically when importing a database through Excel, deleting it, and re-importing a modified version of the file.

In the PR version: If you delete the activity directly, no change, everything goes smoothly. If you delete the database, the group_table and groupdependency entries are still there, thus still creating the issue.

Another minor note, I had to install presamples to be able to launch AB from this PR.

[marc-vdm]

@MaximeAgez Thanks a lot for taking the time and having a look! This is excellent :). I'm just not entirely sure what we should be doing better in this case to fix this.

@cmutel Is there documentation available on how to properly remove things in Brightway? This PR is about removing parameters (or databases containing parameters), but I think there are similar issues with how AB removes projects (that don't actually end up being deleted). If we could review some docs we can review AB code to see where to improve.