Skip to content
This repository has been archived by the owner on Nov 10, 2020. It is now read-only.

Explore > Exports #936

Merged
merged 5 commits into from Dec 1, 2015
Merged

Explore > Exports #936

merged 5 commits into from Dec 1, 2015

Conversation

gemfarmer
Copy link
Contributor

For #858, #685, #681 (code, design, wireframe)

Preview: https://federalist.18f.gov/preview/18F/doi-extractives-data/exports/explore/exports

It is good to go! I used gdp as a template and referred to wireframes in #681.

Questions/todos remaining:

  • I did not address the $/% selector. I will do that in a separate PR
  • The wireframes did not include the Resource column, so I did not bother adding it. Is this on purpose, or unintentional, @shawnbot
  • Nationwide totals not accurate. Currently they are actually the same values as Alabama (first alphabetically), but this could be remedied if we found data for US totals ($) and shares (%) by year.

@Isabelle1512, @shawnbot suggested that I talk to you about holes in the exports data.
What's going on: we don't have nationwide totals, so we can't accurately asses the Nationwide $ total or % of total exports.
Do you know where we can get Nationwide total $s and % ? I looked in the executive summary and only found values for specific years and specific industries.

@meiqimichelle Should the problem above be treated as a bug or blocker? We can merge this and file the Nationwide values as separate bugs to be addressed separately. Or we could wait for the data

return req;
};

function getDataURL(state) {

Choose a reason for hiding this comment

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

'state' is defined but never used.

@shawnbot
Copy link
Contributor

shawnbot commented Dec 1, 2015

This looks great, @gemfarmer. A couple of things, though:

  • I think that the numbers in the TSV are expressed in millions, so we need to multiply them by 1e6 when creating the data files.

  • the nationwide total doesn't appear to be correct:

    image

@@ -95,7 +95,8 @@ async.waterfall([
rows.forEach(function(d, i) {
if (i === 0) console.warn(d, years);
years.forEach(function(year) {
var value = +d['val' + year];
var value = +d['val' + year] * 1e6;
var share = +d['share' + year.substr(-2)] / 100

Choose a reason for hiding this comment

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

Missing semicolon.

Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@gemfarmer gemfarmer changed the title Explore > Exports [WIP] Explore > Exports Dec 1, 2015
@meiqimichelle
Copy link
Contributor

@gemfarmer I say merge this for now and file the nationwide issue as a separate bug to make sure we are pushing as much as we can out for review.

@gemfarmer
Copy link
Contributor Author

Ok. I'll merge and make a separate issue detailing the issue. !

@gemfarmer gemfarmer changed the title [WIP] Explore > Exports Explore > Exports Dec 1, 2015
gemfarmer added a commit that referenced this pull request Dec 1, 2015
@gemfarmer gemfarmer merged commit 965caee into dev Dec 1, 2015
@shawnbot shawnbot deleted the exports branch December 2, 2015 18:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants