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

Make commuter adrio refactor. #120

Merged
merged 2 commits into from
Jun 20, 2024
Merged

Make commuter adrio refactor. #120

merged 2 commits into from
Jun 20, 2024

Conversation

TJohnsonAZ
Copy link
Contributor

Refactor of ADRIOMakerCensus's _make_commuter_adrio() method to utilize DataFrame.pivot() when converting data into a matrix-like shape.

@JavadocMD
Copy link
Contributor

This is a definite improvement but I feel like we could simplify even further if we can find a way to collapse the county/state cases sooner. Can we return the data from "fetch_commuters()" in a universalized format, such that the adrio no longer cares about the differences between state and county? (Notice that some of the steps are duplicated between the two.)

Also, in "fetch_commuters()" I think we can simplify this logic such that the only cases you need to handle are whether scope.granularity equals state or county, and using scope.get_node_ids() to get away from having to deal with what scope.includes entails. You're accessing too much of the "internals" of these data structures and I think it's making your work harder.

Copy link
Contributor

@JavadocMD JavadocMD left a comment

Choose a reason for hiding this comment

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

Yes! Perfect.

@TJohnsonAZ TJohnsonAZ merged commit c0a31c3 into main Jun 20, 2024
1 check passed
@TJohnsonAZ TJohnsonAZ deleted the make-commuter-refactor branch June 20, 2024 18:31
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