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

Change download method for trajectory_clustering.ipynb's data for container testing #1539

Merged
merged 4 commits into from
Feb 23, 2025

Conversation

taureandyernv
Copy link
Contributor

wget no longer exists in our container. As per our notebook guidelines, for testing, every possible notebook should be testable. As we dropped wget from our container, we need to install it here. This code addition intelligently installs wget if necessary.

wget no longer exists in our container.  As per our notebook guidelines, for testing, every possible notebook should be testable.  As we dropped wget from our container, we need to install it here.  This code addition intelligently installs wget if necessary.
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@github-actions github-actions bot added the Python Related to Python code label Feb 11, 2025
Copy link
Member

@harrism harrism left a comment

Choose a reason for hiding this comment

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

The print("Git not found, installing...") should be print("wget not found, installing...")

Copy link
Contributor

@bdice bdice left a comment

Choose a reason for hiding this comment

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

I would recommend one or more of the following alternatives, to avoid mucking up the notebook with apt commands (which may not be portable if run on non-Debian-based systems):

@taureandyernv
Copy link
Contributor Author

i'll modify to curl. Thanks for the suggestion, @bdice. Thanks for the catch @harrism!

@taureandyernv taureandyernv changed the title Add wget install into trajectory_clustering.ipynb for container testing Change download method for trajectory_clustering.ipynb's data for container testing Feb 12, 2025
@bdice bdice added bug Something isn't working non-breaking Non-breaking change labels Feb 23, 2025
@bdice
Copy link
Contributor

bdice commented Feb 23, 2025

/merge

@rapids-bot rapids-bot bot merged commit 948a1e5 into rapidsai:branch-25.04 Feb 23, 2025
71 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working non-breaking Non-breaking change Python Related to Python code
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

3 participants