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

Calculate distance traveled for CCJPA rides #3261

Merged
merged 4 commits into from
Feb 5, 2024

Conversation

charlie-costanzo
Copy link
Member

@charlie-costanzo charlie-costanzo commented Jan 31, 2024

Description

This PR facilitates the calculation of distance traveled (in miles) between location_name and off_location_name for CCJPA rides in the fct_payments_rides_v2 table. It does this by:

  • Creating an O/D pair matrix for CCJPA stations with the distance between each pair calculated in miles
  • Uploads this as a dbt seed
  • Joins this column to the fct_payments_rides_v2 table
  • Adds necessary documentation for the new seed, and new column in the mart table

Type of change

  • New feature
  • Documentation

How has this been tested?

Locally using dbt

Copy link

github-actions bot commented Jan 31, 2024

Warehouse report 📦

DAG

Legend (in order of precedence)

Resource type Indicator Resolution
Large table-materialized model Orange Make the model incremental
Large model without partitioning or clustering Orange Add partitioning and/or clustering
View with more than one child Yellow Materialize as a table or incremental
Incremental Light green
Table Green
View White

@SorenSpicknall
Copy link
Contributor

SorenSpicknall commented Jan 31, 2024

I've two minds about this one. My initial preference is to spin this new information out into another CCJPA-specific model, or even just an enhanced version of fct_payments_rides_v2, to avoid entrenching a precedent of sparse agency-specific columns in fct_payments_rides_v2. However, I can see an argument for inclusion in fct_payments_rides_v2 from a standpoint of avoiding proliferation of agency-specific models or agency-specific joins, and I don't have a ton of the prior context for how agency-specific information has been treated in payments models in the past.

I'll leave the final decision to you, Charlie, since you have a longer history working on payments models than I do, but from my first-pass standpoint I would prefer this column exist somewhere other than fct_payments_rides_v2 - an intermediate model based on the seed, or an agency-specific model that takes only the CCJPA rows of fct_payments_rides_v2 and adds the new column in that more limited context.

Let me know what you decide to do, and I'll put in a formal review after that decision point.

@charlie-costanzo
Copy link
Member Author

okay @SorenSpicknall I've spent some time thinking about this

I did move the joining of the seed to the intermediate table, int_payments__matched_device_transactions

However, I left the column on fct_payments_rides_v2 and made the column name agency-agnostic (distance_miles), and my thinking for this is two-fold:

  • We actually have precedent in doing this for the column distance_meters on the fct_payments_rides_v2 table — it's essentially the same concept except in meters. It's also only relevant for a couple of agencies for the time being.
  • It is the hope/goal that this is relevant for more agencies soon. This is also the preference of the stakeholders who created this ask.

Let me know if you don't agree with this and I'm happy to reconsider.

Copy link
Contributor

@SorenSpicknall SorenSpicknall left a comment

Choose a reason for hiding this comment

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

I'm happy with this approach. Approved!

@charlie-costanzo charlie-costanzo merged commit eb7d47b into main Feb 5, 2024
4 checks passed
@charlie-costanzo charlie-costanzo deleted the ccjpa-miles-traveled branch February 5, 2024 15:59
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