Improve inconsistency of `Dataset.map` interface for `load_from_cache_file`
COLLABORATOR
Hi, thanks for noticing this! We can't just remove the cache control as this allows us to control where the arrow files generated by the ops are written (cached on disk if enabled or a temporary directory if disabled). The right way to address this inconsistency would be by having `load_from_cache_file=None` by default everywhere.
2023-02-08T23:58:29Z
https://github.com/huggingface/datasets/issues/5514#issuecomment-1423393427
https://api.github.com/repos/huggingface/datasets/issues/5514
IC_kwDODunzps5U10KT
https://api.github.com/repos/huggingface/datasets/issues/comments/1423393427/reactions
2023-02-08T23:58:29Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1423393427
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
Hi! Yes, this seems more plausible. I can implement that. One last thing is the type annotation `load_from_cache_file: bool = None`. Which I then would change to `load_from_cache_file: Optional[bool] = None`.
2023-02-09T08:34:22Z
https://github.com/huggingface/datasets/issues/5514#issuecomment-1423816478
https://api.github.com/repos/huggingface/datasets/issues/5514
IC_kwDODunzps5U3bce
https://api.github.com/repos/huggingface/datasets/issues/comments/1423816478/reactions
2023-02-09T08:34:22Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1423816478
https://avatars.githubusercontent.com/u/22773355?v=4
https://api.github.com/users/HallerPatrick/events{/privacy}
https://api.github.com/users/HallerPatrick/followers
https://api.github.com/users/HallerPatrick/following{/other_user}
https://api.github.com/users/HallerPatrick/gists{/gist_id}
https://github.com/HallerPatrick
HallerPatrick
MDQ6VXNlcjIyNzczMzU1
https://api.github.com/users/HallerPatrick/orgs
https://api.github.com/users/HallerPatrick/received_events
https://api.github.com/users/HallerPatrick/repos
https://api.github.com/users/HallerPatrick/starred{/owner}{/repo}
https://api.github.com/users/HallerPatrick/subscriptions
User
https://api.github.com/users/HallerPatrick
public
CONTRIBUTOR
PR #5515 
2023-02-09T10:05:22Z
https://github.com/huggingface/datasets/issues/5514#issuecomment-1423929385
https://api.github.com/repos/huggingface/datasets/issues/5514
IC_kwDODunzps5U33Ap
https://api.github.com/repos/huggingface/datasets/issues/comments/1423929385/reactions
2023-02-09T10:05:22Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1423929385
https://avatars.githubusercontent.com/u/22773355?v=4
https://api.github.com/users/HallerPatrick/events{/privacy}
https://api.github.com/users/HallerPatrick/followers
https://api.github.com/users/HallerPatrick/following{/other_user}
https://api.github.com/users/HallerPatrick/gists{/gist_id}
https://github.com/HallerPatrick
HallerPatrick
MDQ6VXNlcjIyNzczMzU1
https://api.github.com/users/HallerPatrick/orgs
https://api.github.com/users/HallerPatrick/received_events
https://api.github.com/users/HallerPatrick/repos
https://api.github.com/users/HallerPatrick/starred{/owner}{/repo}
https://api.github.com/users/HallerPatrick/subscriptions
User
https://api.github.com/users/HallerPatrick
public
COLLABORATOR
Yes, `Optional[bool]` is the correct type annotation and thanks for the PR.
2023-02-09T12:55:16Z
https://github.com/huggingface/datasets/issues/5514#issuecomment-1424150564
https://api.github.com/repos/huggingface/datasets/issues/5514
IC_kwDODunzps5U4tAk
https://api.github.com/repos/huggingface/datasets/issues/comments/1424150564/reactions
2023-02-09T12:55:16Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1424150564
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
### Feature request


1. Replace the `load_from_cache_file` default value to `True`. 
2. Remove or alter checks from `is_caching_enabled` logic.

### Motivation

I stumbled over an inconsistency in the `Dataset.map` interface. The documentation (and source) states for the parameter `load_from_cache_file`:
```
load_from_cache_file (`bool`, defaults to `True` if caching is enabled):
    If a cache file storing the current computation from `function`
    can be identified, use it instead of recomputing.
```

1. `load_from_cache_file` default value is `None`, while being annotated as `bool`
2. It is inconsistent with other method signatures like `filter`, that have the default value `True`
3. The logic is inconsistent, as the `map` method checks if caching is enabled through `is_caching_enabled`. This logic is not used for other similar methods.

### Your contribution

I am not fully aware of the logic behind caching checks. If this is just a inconsistency that historically grew, I would suggest to remove the `is_caching_enabled` logic as the "default" logic. Maybe someone can give insights, if environment variables have a higher priority than local variables or vice versa. 

If this is clarified, I could adjust the source  according to the "Feature request" section of this issue.