Latest dill release raises exception
MEMBER
Fixed by:
- #4380 
2022-05-20T17:06:27Z
https://github.com/huggingface/datasets/issues/4379#issuecomment-1133129832
https://api.github.com/repos/huggingface/datasets/issues/4379
IC_kwDODunzps5DijBo
https://api.github.com/repos/huggingface/datasets/issues/comments/1133129832/reactions
2022-05-20T17:06:27Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1133129832
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
Just an additional insight, the latest dill (either 0.3.5 or 0.3.5.1) also broke the hashing/fingerprinting of any mapping function.

For example:
```
from datasets import load_dataset

d = load_dataset("rotten_tomatoes")
d.map(lambda x: x)
```

Returns the standard non-dillable error:
```
Parameter 'function'=<function <lambda> at 0x7fe7d18c9560> of the transform datasets.arrow_dataset.Dataset._map_single couldn't be hashed properly....
```
2022-05-20T17:09:02Z
https://github.com/huggingface/datasets/issues/4379#issuecomment-1133131825
https://api.github.com/repos/huggingface/datasets/issues/4379
IC_kwDODunzps5Dijgx
https://api.github.com/repos/huggingface/datasets/issues/comments/1133131825/reactions
2022-05-20T17:09:02Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1133131825
https://avatars.githubusercontent.com/u/4120639?v=4
https://api.github.com/users/gugarosa/events{/privacy}
https://api.github.com/users/gugarosa/followers
https://api.github.com/users/gugarosa/following{/other_user}
https://api.github.com/users/gugarosa/gists{/gist_id}
https://github.com/gugarosa
gugarosa
MDQ6VXNlcjQxMjA2Mzk=
https://api.github.com/users/gugarosa/orgs
https://api.github.com/users/gugarosa/received_events
https://api.github.com/users/gugarosa/repos
https://api.github.com/users/gugarosa/starred{/owner}{/repo}
https://api.github.com/users/gugarosa/subscriptions
User
https://api.github.com/users/gugarosa
public
NONE
@albertvillanova ExamplesTests.test_run_speech_recognition_seq2seq is in which file?
2022-05-20T23:55:00Z
https://github.com/huggingface/datasets/issues/4379#issuecomment-1133471556
https://api.github.com/repos/huggingface/datasets/issues/4379
IC_kwDODunzps5Dj2dE
https://api.github.com/repos/huggingface/datasets/issues/comments/1133471556/reactions
2022-05-20T23:55:00Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1133471556
https://avatars.githubusercontent.com/u/42706068?v=4
https://api.github.com/users/anivegesana/events{/privacy}
https://api.github.com/users/anivegesana/followers
https://api.github.com/users/anivegesana/following{/other_user}
https://api.github.com/users/anivegesana/gists{/gist_id}
https://github.com/anivegesana
anivegesana
MDQ6VXNlcjQyNzA2MDY4
https://api.github.com/users/anivegesana/orgs
https://api.github.com/users/anivegesana/received_events
https://api.github.com/users/anivegesana/repos
https://api.github.com/users/anivegesana/starred{/owner}{/repo}
https://api.github.com/users/anivegesana/subscriptions
User
https://api.github.com/users/anivegesana
public
MEMBER
Thanks a lot @gugarosa for the insight: we will incorporate it in our CI as regression testing for future dill releases.
2022-05-21T04:34:23Z
https://github.com/huggingface/datasets/issues/4379#issuecomment-1133529723
https://api.github.com/repos/huggingface/datasets/issues/4379
IC_kwDODunzps5DkEp7
https://api.github.com/repos/huggingface/datasets/issues/comments/1133529723/reactions
2022-05-21T04:34:23Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1133529723
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
Hi @anivegesana, that test is in `transformers` library:
- https://github.com/huggingface/transformers/blob/main/examples/pytorch/test_pytorch_examples.py#L449
- https://github.com/huggingface/transformers/blob/main/examples/pytorch/speech-recognition/run_speech_recognition_seq2seq.py 
2022-05-21T04:57:31Z
https://github.com/huggingface/datasets/issues/4379#issuecomment-1133532717
https://api.github.com/repos/huggingface/datasets/issues/4379
IC_kwDODunzps5DkFYt
https://api.github.com/repos/huggingface/datasets/issues/comments/1133532717/reactions
2022-05-21T04:57:31Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1133532717
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
NONE
@albertvillanova

I did a deep dive into @gugarosa's problem and found the issue and it might be related to the one @sgugger discovered. In dill 0.3.5(.1), I created a new `save_function` that fixes a bug in dill that prevented the pickling of recursive inner functions. It was a more complete solution to the problem that `dill._dill.stack` tried to solve in the internal API of dill. Since `dill._dill.stack` was no longer needed, I removed it. Since datasets copies the `save_function` directly from the dill API, it stops working with the new dill version since `dill._dill.stack` is no longer present and the `save_function` has been updated with new code.

https://github.com/huggingface/datasets/blob/95193ae61e92aa537d0c65d37a1fd9d2393aae89/src/datasets/utils/py_utils.py#L607-L678

~If the dill version is below 0.3.5, you should keep this function. If it is after, you would need to update your copy of `save_function` to use the code I introduced, or manually add a `stack` variable to `dill._dill` if it doesn't exist. Fortunately, in any version of Python 3.7+, dictionaries are always in insertion order and dill no longer supports Python 3.6 or older. So, any globals dictionary saved by dill 0.3.5+ will be deterministic given that the version of dill is held constant and this save_function is unnecessary for newer versions of dill.~

Ah. I see what is happening. I guess a different copy of the function code is needed that sorts the global variables by name.

```py
if dill.__version__.split('.') < ['0', '3', '5']:
    # current save_function code inside here
else:
    # new save_function code inside here with the following line inserted after creating the globals
    globs = {k: globs[k] for k in sorted(globs.keys())} 
```

Will look into the test case @sgugger pointed out after that and verify if this is causing the problem.

I am actually looking into rewriting the global variables code in uqfoundation/dill#466 and will keep this in mind and will try to create an easy way to modify the global variables in dill 0.3.6 (for example, sort them by key like datasets does).
2022-05-21T08:28:07Z
https://github.com/huggingface/datasets/issues/4379#issuecomment-1133564249
https://api.github.com/repos/huggingface/datasets/issues/4379
IC_kwDODunzps5DkNFZ
https://api.github.com/repos/huggingface/datasets/issues/comments/1133564249/reactions
2022-05-21T09:14:48Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1133564249
https://avatars.githubusercontent.com/u/42706068?v=4
https://api.github.com/users/anivegesana/events{/privacy}
https://api.github.com/users/anivegesana/followers
https://api.github.com/users/anivegesana/following{/other_user}
https://api.github.com/users/anivegesana/gists{/gist_id}
https://github.com/anivegesana
anivegesana
MDQ6VXNlcjQyNzA2MDY4
https://api.github.com/users/anivegesana/orgs
https://api.github.com/users/anivegesana/received_events
https://api.github.com/users/anivegesana/repos
https://api.github.com/users/anivegesana/starred{/owner}{/repo}
https://api.github.com/users/anivegesana/subscriptions
User
https://api.github.com/users/anivegesana
public
MEMBER
Thanks a lot for your investigation @anivegesana.

Yes, we copied-pasted the old `save_function` function from `dill`, just adding a line to make deterministic the order of global variables `globs`. 

However, this function has changed a lot from version 0.3.5, after your PR (thank you for the fix in recursiveness, indeed):
- uqfoundation/dill#443

We have to address this change.

If finally your PR to sort global variables is merged into dill 0.3.6, that will make our life easier, as the tweak will no longer be necessary. ;)

I have included a regression test so that we are sure future releases of dill do not break `datasets`:
- #4385 
2022-05-21T09:15:31Z
https://github.com/huggingface/datasets/issues/4379#issuecomment-1133585924
https://api.github.com/repos/huggingface/datasets/issues/4379
IC_kwDODunzps5DkSYE
https://api.github.com/repos/huggingface/datasets/issues/comments/1133585924/reactions
2022-05-21T09:15:31Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1133585924
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
NONE
I should note that because Python 3.6 and older are now deprecated and Python 3.7 has insertion order dictionaries, the globals in dill will have a deterministic order, just not sorted. I would still keep it sorted like you have it to help with stability (for example, if someone reorders variables in a file, then sorting the globals would not invalidate the cache.)

It seems that the order is not quite deterministic in IPython. Huggingface datasets seems to do well in Jupyter regardless, so it is not a good idea to remove the sorting. uqfoundation/dill#19
2022-05-21T09:20:39Z
https://github.com/huggingface/datasets/issues/4379#issuecomment-1133586660
https://api.github.com/repos/huggingface/datasets/issues/4379
IC_kwDODunzps5DkSjk
https://api.github.com/repos/huggingface/datasets/issues/comments/1133586660/reactions
2022-05-21T15:53:26Z
https://api.github.com/repos/huggingface/datasets/issues/comments/1133586660
https://avatars.githubusercontent.com/u/42706068?v=4
https://api.github.com/users/anivegesana/events{/privacy}
https://api.github.com/users/anivegesana/followers
https://api.github.com/users/anivegesana/following{/other_user}
https://api.github.com/users/anivegesana/gists{/gist_id}
https://github.com/anivegesana
anivegesana
MDQ6VXNlcjQyNzA2MDY4
https://api.github.com/users/anivegesana/orgs
https://api.github.com/users/anivegesana/received_events
https://api.github.com/users/anivegesana/repos
https://api.github.com/users/anivegesana/starred{/owner}{/repo}
https://api.github.com/users/anivegesana/subscriptions
User
https://api.github.com/users/anivegesana
public
## Describe the bug
As reported by @sgugger, latest dill release is breaking things with Datasets.

```
______________ ExamplesTests.test_run_speech_recognition_seq2seq _______________


self = <multiprocess.pool.ApplyResult object at 0x7fa5981a1cd0>, timeout = None

    def get(self, timeout=None):
        self.wait(timeout)
        if not self.ready():
            raise TimeoutError
        if self._success:
            return self._value
        else:
>           raise self._value
E           TypeError: '>' not supported between instances of 'NoneType' and 'float'
```

