Provide script that downloads netflix prize data and puts it in the right place #56
Conversation
audiodude
changed the title
There was a problem hiding this comment.
This file needs to be merged with main. It should be simple, just keep everything that main added, and add your new dependencies and scripts.
| ruff = "~=0.7" | ||
| tqdm = "~=4.66" | ||
| requests = "*" |
There was a problem hiding this comment.
We should leave the existing requests dependency.
|
|
||
| [dev-packages] | ||
| pytest = "~=8.3" | ||
| pytest-cov = "~=5.0" | ||
|
|
||
| [requires] | ||
| python_version = "3.12" | ||
|
|
||
| [scripts] | ||
| download_data = "python download_data.py" |
There was a problem hiding this comment.
These need to be pipenv run python -m mediabridge.download_data. The pipenv run is needed to make the scripts run in the pipenv environment.
There was a problem hiding this comment.
I would recommend moving these scripts to the top level, not under the mediabridge module. The reasoning is that they are utilities for helping set up the environment, not a part of our software/library.
There was a problem hiding this comment.
This might be a bit overkill, because folks could just run rm -rf data to remove the data dir, and your download script will recreate it right?
| import wget | ||
|
|
||
| """ | ||
| For our scripts to work, we need to download the Netflix prize data. As this file is ~500 MB and the licensing terms are dubious, we decided early on not to include it as part of the repo. It is however available on the Internet Archive here: https://archive.org/details/nf_prize_dataset.tar |
There was a problem hiding this comment.
I think this comment only needs the first paragraph. The rest is a description of how to write this very script.
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| url = "https://archive.org/download/nf_prize_dataset.tar/nf_prize_dataset.tar.gz" |
There was a problem hiding this comment.
Not necessary, but the usual pattern is to do:
def main():
url = ...
...
download_file(...)
extract_file()
if __name__ == "__main__":
main()
The main reason (see what I did there?) is that you might later add logic for handling command line args or other things, that are independent of the functionality of the main() function itself. Also, if you make main() its own function, it can be called from other scripts/modules (if that is ever useful).
| ruff = "~=0.7" | ||
| tqdm = "~=4.66" | ||
| requests = "*" | ||
| wget = "*" |
There was a problem hiding this comment.
Also, please specify a loose version requirement, like the other libraries in this file.
wget = "~=9.9"
Or whatever the current major/minor version is. You can get it off pypi, or with pipenv run pip freeze. Don't specify the patch version (so 9.9, not 9.9.9), so that we are pegged to the major version and the minor version can increment (versus being pegged to the minor version and only the patch version can increment). For more information see https://semver.org/
Fixes #43