Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Commit empty files as regular and warn user #1180

Merged
merged 1 commit into from
Nov 10, 2022
Merged

Conversation

Wauplin
Copy link
Contributor

@Wauplin Wauplin commented Nov 10, 2022

Resolve #946.

With this PR:

  1. Empty files are always uploaded as "regular" (LFS/S3 doesn't work for empty files)
  2. A warning message is triggered when user commits an empty file (cc @adrinjalali: once this is released, no need to warn in skops anymore - except if you want a more detailed message)
    1. Except for .gitkeep files (legit use case)

(cc @SBrandeis who created in issue)

@HuggingFaceDocBuilder
Copy link

HuggingFaceDocBuilder commented Nov 10, 2022

The documentation is not available anymore as the PR was closed or merged.

@codecov
Copy link

codecov bot commented Nov 10, 2022

Codecov Report

Base: 83.93% // Head: 84.02% // Increases project coverage by +0.09% 🎉

Coverage data is based on head (7f062e8) compared to base (7e10549).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1180      +/-   ##
==========================================
+ Coverage   83.93%   84.02%   +0.09%     
==========================================
  Files          43       43              
  Lines        4282     4288       +6     
==========================================
+ Hits         3594     3603       +9     
+ Misses        688      685       -3     
Impacted Files Coverage Δ
src/huggingface_hub/_commit_api.py 92.59% <100.00%> (+0.28%) ⬆️
src/huggingface_hub/repository.py 79.96% <0.00%> (+0.51%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Contributor

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Thanks, this is great!

@Wauplin Wauplin merged commit 0d36cbd into main Nov 10, 2022
@Wauplin Wauplin deleted the 946-fix-commit-empty-file branch November 10, 2022 14:28
if not path.endswith(".gitkeep"):
warnings.warn(
f"About to commit an empty file: '{path}'. Are you sure this is"
" intended ?"
Copy link
Member

Choose a reason for hiding this comment

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

*"intended?" (no space)

for addition in additions:
if addition.upload_info.size == 0:
path = addition.path_in_repo
if not path.endswith(".gitkeep"):
Copy link
Member

Choose a reason for hiding this comment

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

maybe extend to all filenames that start with a dot?

(nit) i dont think it's that uncommon to have empty files

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Uploading an empty file with create_commit fails if the file is empty
4 participants