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

79 update config README #91

Open
wants to merge 2 commits into
base: dev
Choose a base branch
from
Open

79 update config README #91

wants to merge 2 commits into from

Conversation

crispy-wonton
Copy link
Collaborator

@crispy-wonton crispy-wonton commented Dec 4, 2024

Fixes #79

Description

Add new data sources and attributions to config/README

Instructions for Reviewer

During your review, along with general review of readability /clarity etc, please could you check:

  • all links work and take you to the expected dataset
  • descriptions make sense
  • you agree with removing the S3 file column. I removed it because I don't think it added value as all the S3 paths can already be found in the config/base.yaml and also because they are so changeable it would create unnecessary work to maintain the README.
  • all the appropriate data attribution statements are there
  • i haven't missed any datasets that should have been added

Checklist:

  • I have refactored my code out from notebooks/
  • I have checked the code runs
  • I have tested the code
  • I have run pre-commit and addressed any issues not automatically fixed
  • I have merged any new changes from dev
  • I have documented the code
    • Major functions have docstrings
    • Appropriate information has been added to READMEs
  • I have explained this PR above
  • I have requested a code review

- add sources for feature and reweighting datasets
- add required data attribution statements
- remove 'S3 file' column for brevity and easier maintenance - the path is too changeable and it is already recorded in the base config file
- add new 'feature' column to feature datasets table
- update config key for NRS_households (from NRS_dwellings) to reflect actual dataset- we use the household not dwelling counts
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.

Add Scottish datasets to README
1 participant