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.

Sign up for GitHub

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

Jump to bottom

Fix JHU's fips-population source file #301

Merged
merged 2 commits into from
Oct 13, 2020
Merged

Fix JHU's fips-population source file #301

merged 2 commits into from
Oct 13, 2020

Conversation

dshemetov
Copy link
Contributor

  • use geocode utility source file and remove old static source file
  • update tests
  • update validation notebook to verify prop signals in NYC

@dshemetov
Fix JHU's fips-population source file
12aa17d 
* use geocode utility source file and remove old static source file
* update tests
* update validation notebook to verify prop signals in NYC
@krivard krivard changed the base branch from main to deploy-jhu October 9, 2020 13:30
@krivard
Copy link
Contributor

krivard commented Oct 9, 2020

It could've gone either way, but I switched the base to deploy-jhu because this PR primarily concerns the JHU indicator. Once we merge it, we'll propagate the changes to main.

sarah-colq
sarah-colq reviewed Oct 13, 2020
View reviewed changes
testing_utils/indicator_validation.ipynb
{
"output_type": "display_data",
"data": {
"text/plain": "<Figure size 1152x432 with 1 Axes>",
Copy link

Choose a reason for hiding this comment

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

Should we be able to see the remote line or are they identical here? I cannot see the remote line on any of the new charts.

sarah-colq
sarah-colq suggested changes Oct 13, 2020
View reviewed changes
Copy link

@sarah-colq sarah-colq left a comment

Choose a reason for hiding this comment

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

Please check the charts in the python notebook. Otherwise, looks good! :)

@dshemetov
Copy link
Contributor Author

dshemetov commented Oct 13, 2020 •
edited
Loading

That notebook isn't really supposed to be tracked. It was meant to be a template, but I kept working in the template and committed changes. Thanks for catching that! I've reverted it back to its template version and renamed it accordingly.

@krivard krivard merged commit 7ebc144 into deploy-jhu Oct 13, 2020
@dshemetov dshemetov mentioned this pull request Oct 18, 2020
Closed
@krivard krivard deleted the jhu_nyc_population_fix branch October 29, 2020 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sarah-colq sarah-colq sarah-colq requested changes

@krivard krivard krivard approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dshemetov @krivard @sarah-colq