LCA-ActivityBrowser / activity-browser

GUI for brightway2
GNU Lesser General Public License v3.0
134 stars 50 forks source link

Missing "name" attribute in exchanges created with Activity Browser #1242

Open raphaeljolivet opened 4 months ago

raphaeljolivet commented 4 months ago

Updating AB

What happened?

When creating exchanges with ActivityBrowser, they miss the attribute "name" in ._data. This field is present for all exchanges of all activities imported with ecoinvent. Isn't it a mandatory attribute in Brightway2 ?

As a consequence, it breaks when used in lca_algebraic

Relevant errors

No response

Operating system

Linux/Other (please specify above)

Conda environment

No response

marc-vdm commented 4 months ago

Hey, thanks for opening this issue.

You're right, AB does currently not add a name field to an exchange, but they do seem to be automatically written on ecoinvent imports (for internal reference, they are not generated on imports of excel databases either, so this seems to be exclusive to ecoinvent).

The following code handles adding an exchange in AB, though note that we are working on updating the controllers right now (#1241): https://github.com/LCA-ActivityBrowser/activity-browser/blob/ffa5ad8495fcf3743a0ffe5b56cc225dd0d6b7d6/activity_browser/controllers/activity.py#L417-L451

Isn't it a mandatory attribute in Brightway2 ?

Judging by that we've never done this, it seems it is not mandatory. However, now that I'm thinking about it it would make a lot of sense if we do it, I can imagine this to be required once we implement multifunctionality properly though, and implementing this seems very easy.

I think it'd be good if we discuss this internally and see how we can best work with this.

That said, this doesn't immediately fix your problem, these fields don't exist. Even if/when we'd implement this, the field would only be applied to new exchanges, which would probably not be helpful for you at all, correct?

As multifunctionality is currently barely present in Brightway (and at least in AB, not implemented), I think it's safe to assume that the input product name is the exchange name? It could be an option to if exc.get('name') fails, that you instead read the exc.input.get("reference product")?

We're happy to hear other suggestions or ideas though!

raphaeljolivet commented 4 months ago

I sure. I was planning to do something like this.

I just wanted to share this issue with you so we could check whether it is a mandatory attribute or not.

Thanks for this feedback.