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.

Sign up for GitHub

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

Jump to bottom

[BE week] - huggingface repo support for datasets_download.py #2130

Merged
merged 23 commits into from
Jul 17, 2023
Merged

[BE week] - huggingface repo support for datasets_download.py #2130

merged 23 commits into from
Jul 17, 2023

Conversation

aclegg3
Copy link
Contributor

@aclegg3 aclegg3 commented Jun 13, 2023 •
edited
Loading

Motivation and Context

To support migration of datasets to huggingface, this PR adds datasets_download.py support for datasources with huggingface repo origins.

The tool clones the repo, supports authentication for private repos, and prunes unneeded asset tracking.

Uses the "version" metadata field to checkout a particular tag on the dataset repo. Only clones that tag without other history.
Updating an existing repo executes a checkout instead of re-cloning.

NOTE: git-lfs and gitpython are required to use this tool. Example install on Ubuntu:

sudo apt install git-lfs
git lfs install
conda install -y gitpython

NOTE: older versions of git-lfs (< 3.0) don't support prune -f option (e.g. Ubuntu 20.04). Use --no-prune for these systems. To check your version:

git --version
git-lfs --version

How Has This Been Tested

CI testing with new public repo sources. Local testing with private repo sources.

TODO:

  • test without lfs installed.

Types of changes

  • Docs change / refactoring / dependency upgrade
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have completed my CLA (see CONTRIBUTING)
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Jun 13, 2023
0mdc
0mdc approved these changes Jun 13, 2023
View reviewed changes
Copy link
Contributor

@0mdc 0mdc left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you!

src_python/habitat_sim/utils/datasets_download.py Show resolved Hide resolved
src_python/habitat_sim/utils/datasets_download.py Outdated
subprocess.check_call(shlex.split(clone_command))

# prune the repo to reduced wasted memory consumption
prune_command = "git lfs prune -f --recent"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

jturner65
jturner65 approved these changes Jul 5, 2023
View reviewed changes
Copy link
Contributor

@jturner65 jturner65 left a comment

Choose a reason for hiding this comment

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

LGTM

@aclegg3
Merge remote-tracking branch 'origin/main' into datasets_download-hug…
79b84e7 
…gingface
@aclegg3
add ReplicaCAD public HF repos. Add tag cloning instead of main.
3d851a5
@aclegg3
add git-lfs to requirements.txt
49d6328
@aclegg3
install lfs before using datasets_download.py
6106eff
@aclegg3
install git on CI machine
566fa6d
@aclegg3
guard clean against missing symbolic link or directory. Add special h…
d5e45a6 
…andlig for repo datasources: no version in dir name, fetch checkout instead of delete and re-download.
@aclegg3
install git-lfs before downloading data
1557fb1
@aclegg3
install gitpython
74da7f7
@aclegg3
conda install -y
759d72f
@aclegg3
try to upgrade git on CI
afa42c3
@aclegg3
make prune optional for compat. with older CI machine git version
76b7cbe
@aclegg3
update downloader to reference new HF datasources
f58b619
@aclegg3
test against lab branch
54b7407
@aclegg3
fetch all branches in lab clone for testing
67c95a4
@aclegg3
refactor repo handling to replace symblink as necessary when checking…
0e9277e 
… out new data
@aclegg3 aclegg3 marked this pull request as ready for review July 12, 2023 21:58
@aclegg3 aclegg3 mentioned this pull request Jul 12, 2023
Merged
2 tasks
0mdc
0mdc approved these changes Jul 13, 2023
View reviewed changes
Copy link
Contributor

@0mdc 0mdc left a comment

Choose a reason for hiding this comment

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

Awesome work! I left some minor comments.

src_python/habitat_sim/utils/datasets_download.py Outdated Show resolved Hide resolved
src_python/habitat_sim/utils/datasets_download.py Outdated Show resolved Hide resolved
src_python/habitat_sim/utils/datasets_download.py Show resolved Hide resolved
src_python/habitat_sim/utils/datasets_download.py Show resolved Hide resolved
jimmytyyang
jimmytyyang approved these changes Jul 13, 2023
View reviewed changes
Copy link
Contributor

@jimmytyyang jimmytyyang left a comment

Choose a reason for hiding this comment

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

LGTM!

@aclegg3 aclegg3 merged commit 2419cf6 into main Jul 17, 2023
@aclegg3 aclegg3 deleted the datasets_download-huggingface branch July 17, 2023 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0mdc 0mdc 0mdc approved these changes

@jimmytyyang jimmytyyang jimmytyyang approved these changes

@jturner65 jturner65 jturner65 approved these changes

@eundersander eundersander Awaiting requested review from eundersander

Assignees
No one assigned
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aclegg3 @jturner65 @jimmytyyang @0mdc @facebook-github-bot