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

Fix negative contributions grouping under Rest issue #765

Merged
merged 3 commits into from
Apr 21, 2022
Merged

Conversation

nabilahmed739
Copy link
Contributor

Fixes the way bw2-analyzer treats negative contributions on the AB side: brightway-lca/brightway2-analyzer#18
Note: If the above issue is fixed in bw2-analyzer in accordance with the current fix then this change can be reverted after consideration if even the changes in normalize method in multilca class is not required any more.

fixes #739

@coveralls
Copy link

coveralls commented Mar 27, 2022

Coverage Status

Coverage remained the same at 54.301% when pulling 8169a11 on bugfix-739 into be1113a on master.

@marc-vdm
Copy link
Member

Would it be a good idea to add TODOs to places that need to be changed back if this gets changed on the BW side?
Otherwise, looks great!

@marc-vdm marc-vdm added the bug label Mar 29, 2022
@cmutel
Copy link
Contributor

cmutel commented Mar 29, 2022

@nabilahmed739 I think it is awesome that you have submitted the PR. Thanks!

I have merged your PR against bw2-analyzer, I think this is a better way to fix the underlying problem. I will release a new version today or tomorrow.

@nabilahmed739
Copy link
Contributor Author

@cmutel Yes, the change in bw2-analyzer would be perfect!
Could you please let us know the new release version of bw2-analyzer which will be compatible with AB? (Please note that AB has not migrated to bw2.5 yet)

@nabilahmed739
Copy link
Contributor Author

The complete fix for #739 is the combination of bw2-analyzer update to version 0.11.2 and completion of this PR

@marc-vdm
Copy link
Member

@cmutel I see bw2analyzer was updated, can we finalize this PR on AB side?

@cmutel
Copy link
Contributor

cmutel commented Apr 13, 2022

@nabilahmed739 @marc-vdm All bw2analyzer releases are 2 & 2.5 compatible (or at least they should be!). +1 on merging from me.

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.

Negative values in contribution analysis aggregated under Rest
4 participants