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

Adding basic time predictions graph page #824

Merged
merged 28 commits into from
Nov 1, 2023
Merged

Adding basic time predictions graph page #824

merged 28 commits into from
Nov 1, 2023

Conversation

devinmatte
Copy link
Member

@devinmatte devinmatte commented Aug 10, 2023

Motivation

Show how accurate the MBTA time predictions are over time

Fixes #822

Changes

Screenshot 2023-10-29 at 8 00 23 PM

Testing Instructions

@github-actions github-actions bot added backend Change to backend code frontend Change to frontend code labels Aug 10, 2023
@devinmatte
Copy link
Member Author

I plan to add an information section describing the data, but, in the meantime, the rest is ready for review

@PatrickCleary
Copy link
Member

Is there a way we could default this to one number?

I was imagining a series of checkboxes below the graph for each bucket. By default, all are selected and we show the aggregate data.

Then if a user wants to see only certain buckets they can select one or more of the buckets. But the graph would always show one line.

What do you think?
predictions_mockup

@devinmatte
Copy link
Member Author

Yeah I kinda like it. It requires less explanation also I think (still needs some). I'll probably play around with this and see if I like the result

@devinmatte
Copy link
Member Author

@PatrickCleary

Screen.Recording.2023-08-15.at.9.57.21.PM.mov

I like it

@nathan-weinberg nathan-weinberg removed their request for review October 12, 2023 18:29
@devinmatte
Copy link
Member Author

Okay this is now ready for review again 😄

Would love to get any other feedback needed to get this in

Copy link
Contributor

@idreyn idreyn left a comment

Choose a reason for hiding this comment

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

Based on new screenshots, LGTM!

modules/predictions/PredictionsDetails.tsx Outdated Show resolved Hide resolved
modules/predictions/PredictionsDetails.tsx Outdated Show resolved Hide resolved
modules/predictions/PredictionsDetails.tsx Outdated Show resolved Hide resolved
modules/predictions/charts/PredictionsBinsGraph.tsx Outdated Show resolved Hide resolved
modules/predictions/PredictionsDetails.tsx Outdated Show resolved Hide resolved
Copy link
Contributor

@mathcolo mathcolo left a comment

Choose a reason for hiding this comment

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

Just a few more and you're golden!!

modules/predictions/charts/PredictionsBinsGraph.tsx Outdated Show resolved Hide resolved
modules/predictions/PredictionsDetails.tsx Outdated Show resolved Hide resolved
modules/predictions/charts/PredictionsGraphWrapper.tsx Outdated Show resolved Hide resolved
modules/predictions/charts/PredictionsGraphWrapper.tsx Outdated Show resolved Hide resolved
modules/predictions/charts/PredictionsGraphWrapper.tsx Outdated Show resolved Hide resolved
modules/predictions/charts/PredictionsBinsGraphWrapper.tsx Outdated Show resolved Hide resolved
@devinmatte devinmatte merged commit 4cdf766 into main Nov 1, 2023
5 checks passed
@devinmatte devinmatte deleted the time-predictions branch November 1, 2023 00:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend Change to backend code frontend Change to frontend code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show time prediction accuracy
5 participants