brightway-lca / brightway2-io

Importing and exporting for the Brightway LCA framework
BSD 3-Clause "New" or "Revised" License
26 stars 40 forks source link

[Discussion/Feature request] Adding `database` field to Linking Iterables by field on import #240

Closed marc-vdm closed 5 months ago

marc-vdm commented 5 months ago

link_iterable_by_fields is great, and the DEFAULT_FIELDS are usually work great as .. well, a default.

However, in BW25 (and bw2io=0.8.12), it is becoming more intended behaviour to have multiple instances of ecoinvent in one project. As such, having multiple instances of processes that fit the DEFAULT_FIELDS are likely to become more common.

Would it make sense include the database field in DEFAULT_FIELDS?

Of course link_iterable_by_fields has a fields variable you can use yourself, but IMO it would make sense to have databases as a default field.

Please let me know what you all think!

cmutel commented 5 months ago

I didn't understand this at first, as one always needs to specify the database against which one is linking. However, I think the problem that @marc-vdm identified is that your source data could include links to multiple secondary databases, and he wants to be able to avoid incorrect matches.

I am reluctant to add this to the default implementation - it will break more than it fixes in most use cases. But I agree that we should make it more clear how to make changes to the fields passed in as a default, both in the documentation and source code. We could also add a convenience function to make this simpler (now you would have to find the curried strategy by index if using the default strategy list).

marc-vdm commented 5 months ago

For clarity:

problem that @marc-vdm identified is that your source data could include links to multiple secondary databases, and he wants to be able to avoid incorrect matches.

Yes, that's indeed what I meant.

However, we continued some discussion internally over here in parallel, and I agree, this should probably not become a default field in BW.

I'll leave this open if you want to keep this for tracking the documentation improvement, otherwise, just close it.