Skip to content

Register deterministic tokenization for range objects#5947

Merged
TomAugspurger merged 1 commit intodask:masterfrom
Quansight-Labs:tokenize-range
Feb 26, 2020
Merged

Register deterministic tokenization for range objects#5947
TomAugspurger merged 1 commit intodask:masterfrom
Quansight-Labs:tokenize-range

Conversation

@jrbourbeau
Copy link
Copy Markdown
Member

@jrbourbeau jrbourbeau commented Feb 25, 2020

Currently our tokenization of range objects isn't deterministic. For example:

from dask.base import tokenize

for _ in range(3):
    print(tokenize(range(5)))

outputs:

d424666834140c08de99b23f85a7b60c
a3a1bdd9013db6364668f1d224d4cac6
13142f6359eb57df2fc8230942f1ece9

This PR adds a deterministic tokenization for range objects based on their start, stop, and step attributes. With the changes in this PR, the above snippet outputs a consistent hash:

3b69507db0ea8dfcb39cfce774a1df29
3b69507db0ea8dfcb39cfce774a1df29
3b69507db0ea8dfcb39cfce774a1df29
  • Tests added / passed
  • Passes black dask / flake8 dask

@jrbourbeau
Copy link
Copy Markdown
Member Author

For reference, the test failures on the AppVeyor are unrelated to the changes in this PR (xref #5948)

Copy link
Copy Markdown
Member

@quasiben quasiben left a comment

Choose a reason for hiding this comment

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

Looks good! I suppose we could do the same for np.arange i the numpy section -- though I would be hard pressed to come up with a use case

@TomAugspurger TomAugspurger merged commit 33ad832 into dask:master Feb 26, 2020
@TomAugspurger
Copy link
Copy Markdown
Member

I'm not sure the same can be done for np.arange, since we're tokenizing the output of arange, which is a regular ndarray (IIUC).

@jrbourbeau jrbourbeau deleted the tokenize-range branch February 26, 2020 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants