activity-browser icon indicating copy to clipboard operation
activity-browser copied to clipboard

Listing exchange keys not found instead of erroring in SDF

Open romainsacchi opened this issue 3 years ago • 4 comments

https://github.com/LCA-ActivityBrowser/activity-browser/blob/622c70dd4e076573e0c938741bc71034a1a76ccc/activity_browser/bwutils/superstructure/manager.py#L126

Would it be better to print the list of exchanges not found in a popup window and maybe drop them and go on with the calculation, instead of throwing an error? I think the error message may not be super clear to the user, plus it would not know what the faulty exchanges are. I had to modify the code to make it export df to find out what the missing keys were.

romainsacchi avatar Mar 13 '22 08:03 romainsacchi

@nabilahmed739 Is this solved by your work in progress by any chance?

edit: no

marc-vdm avatar Mar 14 '22 08:03 marc-vdm

I receive this error and I have no idea how to fix it. please help

simb-sdu avatar Mar 21 '22 17:03 simb-sdu

@romainsacchi Agree, I think we'll be seeing this issue more and more with more users using scenarios.

If you think the solution you wrote was good, feel free to make it a PR. Otherwise, making it a draft PR so I have an example of your ideas and can make something permanent would be nice. If you don't have the time, I'd be happy to take this over partially, if you can put some mockup in paint of what you'd like to see for example would already be really helpful!

I think there are a few things to consider:

  1. We should run this check when calculate is pressed, as the reference flows can still change after scenarios are loaded. This is probably too expensive to check every time a ref. flow is changed.
  2. Showing all missing exchanges may not be wise, as there could be 100K+, perhaps we should show the top 10 df.head and the total amount len(df[df.loc[:, EXCHANGE_KEYS].notna()]) of broken exchanges. Though a table is still something that will take a lot of space to properly show.
  3. We could perhaps tooltip some troubleshooting suggestions like checking database names (and please suggest more options, you may have some more trouble shooting experience here).

marc-vdm avatar Mar 22 '22 07:03 marc-vdm

@romainsacchi As a temporary fix, we now tell users how many of the total exchanges are broken. https://github.com/LCA-ActivityBrowser/activity-browser/blob/50f856455f556663d2d7f88e8fb86876478a33aa/activity_browser/bwutils/superstructure/manager.py#L127

marc-vdm avatar Apr 21 '22 08:04 marc-vdm

Closing this as resolved in #1025

marc-vdm avatar Sep 14 '23 14:09 marc-vdm