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

Wastewater proxy merge #29

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Wastewater proxy merge #29

wants to merge 11 commits into from

Conversation

ccoxen
Copy link
Collaborator

@ccoxen ccoxen commented Dec 16, 2024

Pull completed wastewater proxy script into main. This script writes all of the industrial wastewater proxies and the nonseptic domestic proxy.

…y and summarized the data that feed into the emis
…ote: there is an issue where the proxy emis do not match with the GHGI emis
…ote: there is an issue where the proxy emis do not match with the GHGI emis
… code is integrated into existing ECHO / GHGRP data allocation
…ndle proxy allocation. meat and poultry is the last remaining proxy sector to finalize
…y complete other than potential geospatial de-deduplication improvements.
@ccoxen ccoxen changed the base branch from v3 to main December 17, 2024 15:06
@ccoxen
Copy link
Collaborator Author

ccoxen commented Dec 17, 2024

I went through the updated script to address merge conflicts with the existing script, it should be ready for merge now.

Copy link
Collaborator

@nkruskamp nkruskamp left a comment

Choose a reason for hiding this comment

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

Looks good Chris. Some of the helper functions would be great to pull out and put in utils and I'd like to clean up the geospatial operations to use geopandas with meter based distances.

return industry_df



def convert_state_names_to_codes(df, state_column):
Copy link
Collaborator

Choose a reason for hiding this comment

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

us_state_to_abbrev <- check out this function, I think John wrote it and this might be a duplicate of that function?

})

# Extract coordinates
echo_coords = np.array([(lat, lon) for lat, lon in
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to revise this using the geospatial functions in geopandas instead of doing it manually. I can advise on it.

dist = np.sqrt((ghgrp_row['latitude'] - echo_row['Facility Latitude'])**2 +
(ghgrp_row['longitude'] - echo_row['Facility Longitude'])**2)
if dist < 0.025 and ghgrp_row['Year'] == echo_row['Year']:
dist = np.sqrt((ghgrp_row['latitude'] - echo_row['latitude'])**2 +
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's fine to continue to use RSS here, but I do wonder for readability if it makes sense to use the CRS defined in config.py (EQ_AREA_CRS = "ESRI:102003") or an equidistant CRS to do calculations in meters? This might also help the clarity of the ECHO vs FRS distance tolerance / filtering above doing it in meters instead of DD.

return result_df


def geocode_address(df, address_column):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Love this. Could you move it to utils.py?

Copy link
Collaborator

@nkruskamp nkruskamp left a comment

Choose a reason for hiding this comment

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

This is a complex one. Let'see how much we can extract common functions and get the outputs wrapped into a set of functions with defined input and output files for clarify.

@@ -1,18 +1,25 @@
# %%
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's get ahead of John's code review an add a header for this file.

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.

2 participants