-
Notifications
You must be signed in to change notification settings - Fork 0
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
Fix spending over time data for animated map #114
Conversation
🦙 MegaLinter status: ✅ SUCCESS
See detailed report in MegaLinter reports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Did you look into why the bucket starts at less than the total? There shouldn't be any infrastructure spending before the infrastructure bill was passed!
There was also a bug in the aggregation math that was adding too little each month and thus the coloring on the map's states didn't match the above-the-fold map at the end of the animation.
Also, do you mind referring me to where you made that change ☝️ ? I thought that was already addressed in #96.
Some cosmetic feedback at your option :-)
@@ -36,7 +36,7 @@ export default function AnimatedTotalSpendingBucket({ | |||
const totalAwardsAtTime: number = Object.values( | |||
spendingAtTimeByState | |||
).reduce((sum, stateResults) => { | |||
sum += stateResults && stateResults.per_capita; | |||
sum += (stateResults && stateResults?.total) || 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sum += (stateResults && stateResults?.total) || 0; | |
sum += stateResults?.total ?? 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good. Did you look into why the bucket starts at less than the total? There shouldn't be any infrastructure spending before the infrastructure bill was passed!
It looks to be a data problem. The data we are seeing in the map is set to { "fiscal_year" : 2021, "month" : 1 }
and in the monthly.spending.json
the earliest I see money in Alaska is month 12, 2016:
{
"per_capita": 0,
"total": 0,
"time_period": {
"fiscal_year": 2016,
"month": 11
}
},
{
"per_capita": 0.4374303699704051,
"total": 320000,
"time_period": {
"fiscal_year": 2016,
"month": 12
}
},
Money in Idaho starts in month 4 of 2008:
{
"per_capita": 0,
"total": 0,
"time_period": {
"fiscal_year": 2008,
"month": 3
}
},
{
"per_capita": 53.97577155279746,
"total": 96458212.19,
"time_period": {
"fiscal_year": 2008,
"month": 4
}
},
On develop it's also showing this in Idaho:
{
"per_capita": 53.97577155279746,
"time_period": {
"fiscal_year": 2008,
"month": 4
}
},
{
"per_capita": 170.90665833083855,
"time_period": {
"fiscal_year": 2008,
"month": 5
}
},
And in Alaska ion develop:
{
"per_capita": 0,
"time_period": {
"fiscal_year": 2016,
"month": 11
}
},
{
"per_capita": 0.4374303699704051,
"time_period": {
"fiscal_year": 2016,
"month": 12
}
},
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jacobtylerwalls if we are seeing this on develop, is it worth spinning that out to its own card?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that's wise. The spending codes "Z" and "1" are supposed to exclusively deal with BIL spending, so we need an investigation card to see if we botched something or if the API is returning bad results (or if a rogue agency submitted bad contract data, hah). Would you mind opening the card?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not at all. Will do so first thing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow-up card makes sense for the starting level in the bucket. Nice work.
87793b0
to
176adf0
Compare
Overview
These changes fix how spending over months for each state is calculated and adds a total spending amount to get the AnimatedTotalSpendingBucket to "drain" correctly. Before, the aggregated spending was per capita by state and the total it was subtracting from was the raw total in the BIL, meaning it didn't go down by very much at all.
There was also a bug in the aggregation math that was adding too little each month and thus the coloring on the map's states didn't match the above-the-fold map at the end of the animation.
Closes #104
Demo
Notes
Not sure if it's because of the particular start time we are using but the bucket starts at 418B instead of 550B
Testing Instructions
src/app/sr/data/monthly.spending.json
(This may be unnecessary but the fetch data script would skip if the file existed, unless there is a flag I didn't find, which, if so, do that instead)./scripts/fetch-data
(I sometimes had to do this twice because of an exit with code 1, but no output?)./scripts/server
and navigate to the homepageChecklist
fixup!
commits have been squashedCHANGELOG.md
updated with summary of features or fixes, following Keep a Changelog guidelinesREADME.md
updated if necessary to reflect the changes