Delete extracted files when loading dataset
MEMBER
Sure @stas00, it is still a draft pull request. :)
2021-07-12T21:11:15Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-878598412
https://api.github.com/repos/huggingface/datasets/issues/2631
MDEyOklzc3VlQ29tbWVudDg3ODU5ODQxMg==
https://api.github.com/repos/huggingface/datasets/issues/comments/878598412/reactions
2021-07-12T21:11:15Z
https://api.github.com/repos/huggingface/datasets/issues/comments/878598412
https://avatars.githubusercontent.com/u/8515462?v=4
https://api.github.com/users/albertvillanova/events{/privacy}
https://api.github.com/users/albertvillanova/followers
https://api.github.com/users/albertvillanova/following{/other_user}
https://api.github.com/users/albertvillanova/gists{/gist_id}
https://github.com/albertvillanova
albertvillanova
MDQ6VXNlcjg1MTU0NjI=
https://api.github.com/users/albertvillanova/orgs
https://api.github.com/users/albertvillanova/received_events
https://api.github.com/users/albertvillanova/repos
https://api.github.com/users/albertvillanova/starred{/owner}{/repo}
https://api.github.com/users/albertvillanova/subscriptions
User
https://api.github.com/users/albertvillanova
public
CONTRIBUTOR
Yes, I noticed it after reviewing - my apologies.
2021-07-12T21:18:44Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-878603240
https://api.github.com/repos/huggingface/datasets/issues/2631
MDEyOklzc3VlQ29tbWVudDg3ODYwMzI0MA==
https://api.github.com/repos/huggingface/datasets/issues/comments/878603240/reactions
2021-07-12T21:18:44Z
https://api.github.com/repos/huggingface/datasets/issues/comments/878603240
https://avatars.githubusercontent.com/u/10676103?v=4
https://api.github.com/users/stas00/events{/privacy}
https://api.github.com/users/stas00/followers
https://api.github.com/users/stas00/following{/other_user}
https://api.github.com/users/stas00/gists{/gist_id}
https://github.com/stas00
stas00
MDQ6VXNlcjEwNjc2MTAz
https://api.github.com/users/stas00/orgs
https://api.github.com/users/stas00/received_events
https://api.github.com/users/stas00/repos
https://api.github.com/users/stas00/starred{/owner}{/repo}
https://api.github.com/users/stas00/subscriptions
User
https://api.github.com/users/stas00
public
MEMBER
The problem with this approach is that it also deletes the downloaded files (if they need not be extracted). 😟 
2021-07-13T08:01:57Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-878868214
https://api.github.com/repos/huggingface/datasets/issues/2631
MDEyOklzc3VlQ29tbWVudDg3ODg2ODIxNA==
https://api.github.com/repos/huggingface/datasets/issues/comments/878868214/reactions
2021-07-13T08:01:57Z
https://api.github.com/repos/huggingface/datasets/issues/comments/878868214
https://avatars.githubusercontent.com/u/8515462?v=4
https://api.github.com/users/albertvillanova/events{/privacy}
https://api.github.com/users/albertvillanova/followers
https://api.github.com/users/albertvillanova/following{/other_user}
https://api.github.com/users/albertvillanova/gists{/gist_id}
https://github.com/albertvillanova
albertvillanova
MDQ6VXNlcjg1MTU0NjI=
https://api.github.com/users/albertvillanova/orgs
https://api.github.com/users/albertvillanova/received_events
https://api.github.com/users/albertvillanova/repos
https://api.github.com/users/albertvillanova/starred{/owner}{/repo}
https://api.github.com/users/albertvillanova/subscriptions
User
https://api.github.com/users/albertvillanova
public
CONTRIBUTOR
> The problem with this approach is that it also deletes the downloaded files (if they need not be extracted). worried

Right! These probably should not be deleted by default, but having an option for those users who are tight on disc space?
2021-07-13T19:02:50Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-879327609
https://api.github.com/repos/huggingface/datasets/issues/2631
MDEyOklzc3VlQ29tbWVudDg3OTMyNzYwOQ==
https://api.github.com/repos/huggingface/datasets/issues/comments/879327609/reactions
2021-07-13T19:02:50Z
https://api.github.com/repos/huggingface/datasets/issues/comments/879327609
https://avatars.githubusercontent.com/u/10676103?v=4
https://api.github.com/users/stas00/events{/privacy}
https://api.github.com/users/stas00/followers
https://api.github.com/users/stas00/following{/other_user}
https://api.github.com/users/stas00/gists{/gist_id}
https://github.com/stas00
stas00
MDQ6VXNlcjEwNjc2MTAz
https://api.github.com/users/stas00/orgs
https://api.github.com/users/stas00/received_events
https://api.github.com/users/stas00/repos
https://api.github.com/users/stas00/starred{/owner}{/repo}
https://api.github.com/users/stas00/subscriptions
User
https://api.github.com/users/stas00
public
MEMBER
> Right! These probably should not be deleted by default, but having an option for those users who are tight on disc space?

I propose leaving that for another PR, and leave this one handling only with "extracted" files. Is it OK for you? :) 
2021-07-15T08:53:40Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-880520283
https://api.github.com/repos/huggingface/datasets/issues/2631
MDEyOklzc3VlQ29tbWVudDg4MDUyMDI4Mw==
https://api.github.com/repos/huggingface/datasets/issues/comments/880520283/reactions
2021-07-15T08:53:40Z
https://api.github.com/repos/huggingface/datasets/issues/comments/880520283
https://avatars.githubusercontent.com/u/8515462?v=4
https://api.github.com/users/albertvillanova/events{/privacy}
https://api.github.com/users/albertvillanova/followers
https://api.github.com/users/albertvillanova/following{/other_user}
https://api.github.com/users/albertvillanova/gists{/gist_id}
https://github.com/albertvillanova
albertvillanova
MDQ6VXNlcjg1MTU0NjI=
https://api.github.com/users/albertvillanova/orgs
https://api.github.com/users/albertvillanova/received_events
https://api.github.com/users/albertvillanova/repos
https://api.github.com/users/albertvillanova/starred{/owner}{/repo}
https://api.github.com/users/albertvillanova/subscriptions
User
https://api.github.com/users/albertvillanova
public
MEMBER
Awesome thanks !
I just have one question: what about image/audio datasets for which we store the path to the extracted file on the arrow data ?
In this case the default should be to keep the extracted files.

So for now I would just make `keep_extracted=True` by default until we have a way to separate extracted files that can be deleted and extracted files that are actual resources of the dataset.
2021-07-15T09:54:40Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-880560766
https://api.github.com/repos/huggingface/datasets/issues/2631
MDEyOklzc3VlQ29tbWVudDg4MDU2MDc2Ng==
https://api.github.com/repos/huggingface/datasets/issues/comments/880560766/reactions
2021-07-15T09:54:40Z
https://api.github.com/repos/huggingface/datasets/issues/comments/880560766
https://avatars.githubusercontent.com/u/42851186?v=4
https://api.github.com/users/lhoestq/events{/privacy}
https://api.github.com/users/lhoestq/followers
https://api.github.com/users/lhoestq/following{/other_user}
https://api.github.com/users/lhoestq/gists{/gist_id}
https://github.com/lhoestq
lhoestq
MDQ6VXNlcjQyODUxMTg2
https://api.github.com/users/lhoestq/orgs
https://api.github.com/users/lhoestq/received_events
https://api.github.com/users/lhoestq/repos
https://api.github.com/users/lhoestq/starred{/owner}{/repo}
https://api.github.com/users/lhoestq/subscriptions
User
https://api.github.com/users/lhoestq
public
MEMBER
@lhoestq, current implementation only deletes extracted "files", not extracted "directories", as it uses: `os.remove(path)`. I'm going to add a filter on files, so that this line does not throw an exception when passed a directory.

For audio datasets, the audio files are inside the extracted "directory", so they are not deleted.
2021-07-16T14:46:48Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-881504607
https://api.github.com/repos/huggingface/datasets/issues/2631
IC_kwDODunzps40irFf
https://api.github.com/repos/huggingface/datasets/issues/comments/881504607/reactions
2021-07-16T14:46:48Z
https://api.github.com/repos/huggingface/datasets/issues/comments/881504607
https://avatars.githubusercontent.com/u/8515462?v=4
https://api.github.com/users/albertvillanova/events{/privacy}
https://api.github.com/users/albertvillanova/followers
https://api.github.com/users/albertvillanova/following{/other_user}
https://api.github.com/users/albertvillanova/gists{/gist_id}
https://github.com/albertvillanova
albertvillanova
MDQ6VXNlcjg1MTU0NjI=
https://api.github.com/users/albertvillanova/orgs
https://api.github.com/users/albertvillanova/received_events
https://api.github.com/users/albertvillanova/repos
https://api.github.com/users/albertvillanova/starred{/owner}{/repo}
https://api.github.com/users/albertvillanova/subscriptions
User
https://api.github.com/users/albertvillanova
public
MEMBER
I'm still more in favor of having `keep_extracted=True` by default:
- When working with a dataset, you call `load_dataset` many times. By default we want to keep objects extracted to not extract them over and over again (it can take a long time). Then once you know what you're doing and you want to optimize disk space, you can do `keep_extracted=False`. Deleting the extracted files by default is a regression that can lead to slow downs for people calling `load_dataset` many times, which is common when experimenting
- This behavior doesn't sound natural as a default behavior. In the rest of the library, things are cached and not removed unless you explicitly say do (`map` caching for example). Moreover the function in the download manager is called `download_and_extract`, not `download_and_extract_and_remove_extracted_files`

Let me know what you think !
2021-07-16T18:28:03Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-881637365
https://api.github.com/repos/huggingface/datasets/issues/2631
IC_kwDODunzps40jLf1
https://api.github.com/repos/huggingface/datasets/issues/comments/881637365/reactions
2021-07-16T18:28:03Z
https://api.github.com/repos/huggingface/datasets/issues/comments/881637365
https://avatars.githubusercontent.com/u/42851186?v=4
https://api.github.com/users/lhoestq/events{/privacy}
https://api.github.com/users/lhoestq/followers
https://api.github.com/users/lhoestq/following{/other_user}
https://api.github.com/users/lhoestq/gists{/gist_id}
https://github.com/lhoestq
lhoestq
MDQ6VXNlcjQyODUxMTg2
https://api.github.com/users/lhoestq/orgs
https://api.github.com/users/lhoestq/received_events
https://api.github.com/users/lhoestq/repos
https://api.github.com/users/lhoestq/starred{/owner}{/repo}
https://api.github.com/users/lhoestq/subscriptions
User
https://api.github.com/users/lhoestq
public
CONTRIBUTOR
I think the main issue is that after doing some work users typically move on to other datasets and the amount of disc space used keeps on growing. So your logic is very sound and perhaps what's really needed is a cleansweep function that can go through **all** datasets and clean them up to the desired degree:

- delete all extracted files
- delete all sources
- delete all caches
- delete all caches that haven't been accessed  in 6 months
- delete completely old datasets that haven't been accessed  in 6 months
- more?

So a user can launch a little application, choose what they want to clean up and voila they have just freed up a huge amount of disc space. Makes me think of Ubuntu Tweak's Janitor app - very useful.

At the moment, this process of linting is very daunting and error-prone, especially due to all those dirs/files with hash names.
2021-07-16T18:38:07Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-881642566
https://api.github.com/repos/huggingface/datasets/issues/2631
IC_kwDODunzps40jMxG
https://api.github.com/repos/huggingface/datasets/issues/comments/881642566/reactions
2021-07-16T18:41:52Z
https://api.github.com/repos/huggingface/datasets/issues/comments/881642566
https://avatars.githubusercontent.com/u/10676103?v=4
https://api.github.com/users/stas00/events{/privacy}
https://api.github.com/users/stas00/followers
https://api.github.com/users/stas00/following{/other_user}
https://api.github.com/users/stas00/gists{/gist_id}
https://github.com/stas00
stas00
MDQ6VXNlcjEwNjc2MTAz
https://api.github.com/users/stas00/orgs
https://api.github.com/users/stas00/received_events
https://api.github.com/users/stas00/repos
https://api.github.com/users/stas00/starred{/owner}{/repo}
https://api.github.com/users/stas00/subscriptions
User
https://api.github.com/users/stas00
public
COLLABORATOR
@stas00 I've had the same idea. Instead of the full-fledged app, a simpler approach would be to add a new command to the CLI.
2021-07-16T19:33:51Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-881672580
https://api.github.com/repos/huggingface/datasets/issues/2631
IC_kwDODunzps40jUGE
https://api.github.com/repos/huggingface/datasets/issues/comments/881672580/reactions
2021-07-16T19:33:51Z
https://api.github.com/repos/huggingface/datasets/issues/comments/881672580
https://avatars.githubusercontent.com/u/47462742?v=4
https://api.github.com/users/mariosasko/events{/privacy}
https://api.github.com/users/mariosasko/followers
https://api.github.com/users/mariosasko/following{/other_user}
https://api.github.com/users/mariosasko/gists{/gist_id}
https://github.com/mariosasko
mariosasko
MDQ6VXNlcjQ3NDYyNzQy
https://api.github.com/users/mariosasko/orgs
https://api.github.com/users/mariosasko/received_events
https://api.github.com/users/mariosasko/repos
https://api.github.com/users/mariosasko/starred{/owner}{/repo}
https://api.github.com/users/mariosasko/subscriptions
User
https://api.github.com/users/mariosasko
public
CONTRIBUTOR
oh, CLI would be perfect. I didn't mean to request a GUI-one specifically, was just using it as an example.

One could even do a crontab to delete old datasets that haven't been accesses in X months.
2021-07-16T19:43:41Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-881677370
https://api.github.com/repos/huggingface/datasets/issues/2631
IC_kwDODunzps40jVQ6
https://api.github.com/repos/huggingface/datasets/issues/comments/881677370/reactions
2021-07-16T19:43:41Z
https://api.github.com/repos/huggingface/datasets/issues/comments/881677370
https://avatars.githubusercontent.com/u/10676103?v=4
https://api.github.com/users/stas00/events{/privacy}
https://api.github.com/users/stas00/followers
https://api.github.com/users/stas00/following{/other_user}
https://api.github.com/users/stas00/gists{/gist_id}
https://github.com/stas00
stas00
MDQ6VXNlcjEwNjc2MTAz
https://api.github.com/users/stas00/orgs
https://api.github.com/users/stas00/received_events
https://api.github.com/users/stas00/repos
https://api.github.com/users/stas00/starred{/owner}{/repo}
https://api.github.com/users/stas00/subscriptions
User
https://api.github.com/users/stas00
public
MEMBER
@lhoestq I totally agree with you. I'm addressing that change.

@stas00, @mariosasko, that could eventually be addressed in another pull request. The objective of this PR is:
- add an option to pass to `load_dataset`, so that extracted files are deleted
- do this deletion file per file, once the file has been already used to generate the cache Arrow file
2021-07-19T08:11:48Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-882341252
https://api.github.com/repos/huggingface/datasets/issues/2631
IC_kwDODunzps40l3WE
https://api.github.com/repos/huggingface/datasets/issues/comments/882341252/reactions
2021-07-19T08:11:48Z
https://api.github.com/repos/huggingface/datasets/issues/comments/882341252
https://avatars.githubusercontent.com/u/8515462?v=4
https://api.github.com/users/albertvillanova/events{/privacy}
https://api.github.com/users/albertvillanova/followers
https://api.github.com/users/albertvillanova/following{/other_user}
https://api.github.com/users/albertvillanova/gists{/gist_id}
https://github.com/albertvillanova
albertvillanova
MDQ6VXNlcjg1MTU0NjI=
https://api.github.com/users/albertvillanova/orgs
https://api.github.com/users/albertvillanova/received_events
https://api.github.com/users/albertvillanova/repos
https://api.github.com/users/albertvillanova/starred{/owner}{/repo}
https://api.github.com/users/albertvillanova/subscriptions
User
https://api.github.com/users/albertvillanova
public
MEMBER
I also like the idea of having a CLI tool to help users clean their cache and save disk space, good idea !
2021-07-19T09:02:26Z
https://github.com/huggingface/datasets/pull/2631#issuecomment-882377163
https://api.github.com/repos/huggingface/datasets/issues/2631
IC_kwDODunzps40mAHL
https://api.github.com/repos/huggingface/datasets/issues/comments/882377163/reactions
2021-07-19T09:02:26Z
https://api.github.com/repos/huggingface/datasets/issues/comments/882377163
https://avatars.githubusercontent.com/u/42851186?v=4
https://api.github.com/users/lhoestq/events{/privacy}
https://api.github.com/users/lhoestq/followers
https://api.github.com/users/lhoestq/following{/other_user}
https://api.github.com/users/lhoestq/gists{/gist_id}
https://github.com/lhoestq
lhoestq
MDQ6VXNlcjQyODUxMTg2
https://api.github.com/users/lhoestq/orgs
https://api.github.com/users/lhoestq/received_events
https://api.github.com/users/lhoestq/repos
https://api.github.com/users/lhoestq/starred{/owner}{/repo}
https://api.github.com/users/lhoestq/subscriptions
User
https://api.github.com/users/lhoestq
public
Close #2481, close #2604, close #2591.

cc: @stas00, @thomwolf, @BirgerMoell 