-
Notifications
You must be signed in to change notification settings - Fork 29
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
refactoring scrapers/package.py #81
Conversation
also happy thanksgiving btw :P |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me; thanks for the PR (and sorry for breaking your previous one :/).
I've come around to agreeing with you that it would be good to split off the overrides into a separate file that is in an actual data format rather than being a Python module.
Part of me still does want to use TOML instead of JSON, though, to make the overrides file more easily human-writable. Since Python 3.11 comes with a TOML reader, I feel like TOML is mainstream enough now that we wouldn't have to worry about it being too bespoke (but we would need to use an external package to maintain compatibility with older Python versions). What are your thoughts?
Happy Thanksgiving to you too!
This does three things:
scrapers/package.py
extensively by splitting off therun()
function into two helper functions, namelyload_json_data
andmerge_datasets
.overrides.json
file. (Previously, in Move overrides from scrapers/package.py into a separate file, scrapers/overrides.json #76 there had been a substantial set of overrides, but since the resulting code didn't work due to further commits upstream, I abandoned it.)README.md
accordingly