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

Feat/ecoinvent interface #16

Merged
merged 16 commits into from
Jul 1, 2024
Merged

Feat/ecoinvent interface #16

merged 16 commits into from
Jul 1, 2024

Conversation

will7200
Copy link
Collaborator

@will7200 will7200 commented Jun 6, 2024

  • Integrates ecoinvent_interface package instead of eidl.
  • Uses a versioned biosphere database instead of the default batabase named biosphere

Closes #7

Checklist

  • Keep pull requests small so they can be easily reviewed.
  • Update the documentation, please follow the numpy style guide.
  • Update tests.
  • Categorize the PR by setting a good title and adding one of the labels:
    bug, feature, ui, change, documentation, breaking, ci
    as they show up in the changelog.
  • Link this PR to related issues by using closing keywords.
  • Add a milestone to the PR for the intended release.
  • Request a review from another developer.

@will7200 will7200 self-assigned this Jun 6, 2024
@jsvgoncalves jsvgoncalves requested a review from cmutel June 6, 2024 09:23
Copy link

@cmutel cmutel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, I am impressed that you saw some basic patterns have changed.

In the future I am pretty sure we can remove ABEcoinventDownloader completely.

One last change needed (building on LCA-ActivityBrowser#1301): The current code assumes that a database called "biosphere3" is present, but this is an antipattern. Instead, we should set the default biosphere preference to the biosphere database corresponding to the ecoinvent import. So, if we imported ecoinvent 3.9.1, we would end up with two databases: ecoinvent-3.9.1-something and ecoinvent-3.9.1-biosphere. We need to run the following:

import bw2data as bd
bd.preferences['biosphere_database'] = 'ecoinvent-3.9.1-biosphere'
bd.preferences.flush()

@will7200
Copy link
Collaborator Author

will7200 commented Jun 7, 2024

Added the bd.preferences to update on eco version page.
Do we need to run UpdateBiosphere?

@cmutel
Copy link

cmutel commented Jun 7, 2024

Do we need to run UpdateBiosphere?

No. In general this isn't a great idea, and should require manual action by the user after the import complete.

@will7200 will7200 marked this pull request as ready for review June 9, 2024 03:59
@will7200
Copy link
Collaborator Author

will7200 commented Jun 9, 2024

I ended having to integrate UpdateBiosphere for the newly versioned database created. Otherwise the end user will go through the flow of trying to import the database, only to get the update the biosphere by using 'File' -> 'Update biosphere...' message, having to go through the wizard twice to get it to work properly.

This versioned database page only gets shown to the end user if that particular database does not exist already.

@will7200 will7200 requested a review from cmutel June 11, 2024 05:08
@cmutel cmutel merged commit d133328 into brightway25 Jul 1, 2024
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.

3 participants