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

Add HRP countries to population theme #71

Merged
merged 7 commits into from
Jan 30, 2024
Merged

Add HRP countries to population theme #71

merged 7 commits into from
Jan 30, 2024

Conversation

b-j-mills
Copy link
Collaborator

I haven't figured out how to address the tests yet - I was planning to limit the tests to the initial six countries, unless I should include all 25 of them. I just thought that number of files would be overwhelming and the tests would take a very long time to run.

@b-j-mills b-j-mills requested a review from mcarans January 26, 2024 21:00
Copy link

github-actions bot commented Jan 26, 2024

Test Results

4 tests  ±0   4 ✅ ±0   4m 35s ⏱️ +8s
1 suites ±0   0 💤 ±0 
1 files   ±0   0 ❌ ±0 

Results for commit 0ed9552. ± Comparison against base commit d725b2b.

♻️ This comment has been updated with latest results.

"NGA",
"TCD",
"YEM",
]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I included this to try to control which countries are run in the scraper, but it's not having the effect I hoped for. The tests fail looking for data for CMR.
@mcarans is there a way to limit the countries I'm running like in the covid viz scraper?

Copy link
Contributor

Choose a reason for hiding this comment

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

This will require making a change to HDX Python Scraper to allow limiting the countries

@coveralls
Copy link

coveralls commented Jan 29, 2024

Pull Request Test Coverage Report for Build 7717136522

  • 0 of 17 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.1%) to 93.409%

Totals Coverage Status
Change from base Build 7716520285: 0.1%
Covered Lines: 907
Relevant Lines: 971

💛 - Coveralls

@b-j-mills b-j-mills marked this pull request as ready for review January 29, 2024 23:30
project_config_dict = subset_scraper_countries(
project_config_dict,
countries_to_remove,
)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mcarans I created a function "subset_scraper_countries" to remove scrapers from the configs as a workaround to changing hdx-python-scraper. It feels like adding this functionality to hdx-python-scraper isn't worth the number of times we would use it, but if you think it would be useful I'm happy to look into it!

Copy link
Contributor

@mcarans mcarans left a comment

Choose a reason for hiding this comment

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

This looks fine. How scalable do you think the process is? Is it a lot of work for all the countries?

As for adding a function to hdx-python-scraper, I think what you wanted is likely to be needed again although rather than deleting keys, I'd add a parameter somewhere to specify desired countries. I'll look into this sometime.

@b-j-mills
Copy link
Collaborator Author

Sounds good! I'll create a ticket and add a to-do in the code.

For the themes we currently have it's definitely scaleable (except for the HNO data). They're either global data that we can just change the filter on, or similar enough between countries that adding more resources will be straightforward.

@b-j-mills b-j-mills merged commit 3a4b255 into main Jan 30, 2024
3 checks passed
@turnerm turnerm deleted the HAPI-332 branch May 27, 2024 13:05
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.

3 participants