USEPA / standardizedinventories

Standardized Release and Waste Inventories
MIT License
25 stars 16 forks source link

Updates related to filtering processed inventories #81

Closed bl-young closed 2 years ago

bl-young commented 2 years ago
WesIngwersen commented 2 years ago

This is looking good. I would recommend moving filter.yaml up a level to be at the same level as the config.yaml The levels in filter.yaml are a little confusing - it's hard to detect any clear pattern and there seems to be some entries strictly for metadata, and filter.py is similiar in that there is a general filter_inventory function but there are some filters that don't use it. However I understand that this is a work in progress.

bl-young commented 2 years ago

This is looking good. I would recommend moving filter.yaml up a level to be at the same level as the config.yaml The levels in filter.yaml are a little confusing - it's hard to detect any clear pattern and there seems to be some entries strictly for metadata, and filter.py is similiar in that there is a general filter_inventory function but there are some filters that don't use it. However I understand that this is a work in progress.

@WesIngwersen I've addressed these issues (specifically https://github.com/USEPA/standardizedinventories/commit/19fb8e896e249b3a036c84b6fec30645e96e3c8a and https://github.com/USEPA/standardizedinventories/commit/4deadb5c9b14400583c9d11722ab703379a42644). On filter.yaml, my intention was to provide a description of each filter, as well as any parameters if necessary. So yes it is combined metadata and parameters. I am open to alternatives, but wanted to keep everything in one spot.

bl-young commented 2 years ago

@WesIngwersen I'd like to pull this in and do a 0.9.9 release so that I can begin final testing for v1.0 release.

WesIngwersen commented 2 years ago

Is filter_for_LCI intentionally left in as a separate parameter than filter_list? Is that mainly to make it more backward compatible?

bl-young commented 2 years ago

Is filter_for_LCI intentionally left in as a separate parameter than filter_list? Is that mainly to make it more backward compatible?

correct - I didn't want to remove that parameter yet. I'm thinking its probably used in eLCI

bl-young commented 2 years ago

@WesIngwersen the only difference in your two filters above is that "filter_for_LCI" will also remove some flows. For TRI this is just "Mixture" and "Trade Secret Chemical". The EPA rep told me yesterday that he thought those two flows were also accidentally dropped along with the compounds. So in fact, your results may be correct. But the point stands and we can do a little more testing of the filters.