Skip to content

New chart items#33

Open
iansargent wants to merge 11 commits into
mainfrom
new-chart-items
Open

New chart items#33
iansargent wants to merge 11 commits into
mainfrom
new-chart-items

Conversation

@iansargent

Copy link
Copy Markdown
Collaborator
  • Added 3 new TrendChart items: UnemploymentTrendChart, EarningsTrendChart, and MedianAgeTrend Chart
  • New aggregation logic if profile interest is set to VT statewide (still needs some tweaking)
  • New api calls for special time series charts
  • Documented new charts to the plots.md file

Comment thread backend/api/routes/post_routes/post_acs5_db.py
[request.name, request.year_min, request.year_max],
).df()
# If requesting Vermont (state-level), aggregate all counties
if request.name.lower() == "vermont":

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you update these (and all other SQL queries here) to match the paradigm for the zoning dataset?

That is, move the actual logic here into a subsection of the "serve" folder, with the SQL pumped in from .sql files (and ideally parameterized a bit better, with less repetition).

like a lot of these you could just make table a parameter you pass in. so that its "FROM {table}" or "FROM ?".

@FWJK1 FWJK1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

  • Left comments in the post_acs5_db that apply to the post_qcew as well.
  • overall good!
  • TO fix:
  • take the chance to port the logic over to the 'build -> serve -> API' model.
  • parameterize repetive SQL and put in .sql files. See backend/serve/zoning.

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