search

brightway-lca / brightway2-calc

The calculation engine for the Brightway2 life cycle assessment framework.
BSD 3-Clause "New" or "Revised" License
14 stars 16 forks source link

Document/Clarify Removal of GraphTraversal Functionality #91

Open michaelweinold opened 8 months ago

michaelweinold commented 8 months ago

@cmutel, you moved (some?) graph traversal functionality to the bw_graph_tools package with https://github.com/brightway-lca/brightway2-calc/commit/273c5209ba22f9ddae2ef7e060bd7de0b9d27a8e. This is not documented in the changelog (or anywhere else except in the commit title). The last changelog entry related to graph traversal simply mentions:

Changed GraphTraversal to AssumedDiagonalGraphTraversal

This means that on the current main branch, there is no graph traversal functionality in the bw2calc package at all.

However, in the latest dev version, there is a bw2calc/graph_traversal.py file . bw_graph_tools is not called anywhere.

Is there some reason to this madness 🙈?

cmutel commented 8 months ago

However, in the latest dev version, there is a bw2calc/graph_traversal.py file

You posted a link to 2.0.dev12 - the latest dev release is 2.0.dev16 :)

michaelweinold commented 8 months ago

...oh - it seems that GH can't sort tags properly:

Screenshot 2024-03-06 at 08 14 45

But in any case, the file is also present in the dev16 version.

cmutel commented 8 months ago

You're right, sorry about that. The DEV16 release was on October 22, and the graph traversal was removed on October 26.

Brightway2 is pinned to bw2calc < 2.0.

On the current version of Brightway25 (dev), how should one use the GraphTraversal class? Call it from bw2calc? What is the role of bw_graph_tools in this case?

One should only use bw_graph_tools, this is strictly better in every way than the old graph traversal class.

michaelweinold commented 8 months ago

Ok, thank you for the clarification. Can I therefore:

?

cmutel commented 8 months ago

Changelog update in https://github.com/brightway-lca/brightway2-calc/commit/1dd1132d92eefe81b6444c8243785dccbd71fa82. But the main branch isn't ready for a release yet (yes, yes, this should have been a feature branch and not merged to main...).

So I think for now you will just need to manually use bw_graph_utils instead.

michaelweinold commented 8 months ago

Fine, we'll keep this open until then. Thank you! I will update the documentation of the bw_graph_tools package in the coming weeks, as I go through the code in detail.