Change doctests to Python 3 #3690 by eric-bonfadini · Pull Request #3744 · dask/dask

eric-bonfadini · 2018-07-10T21:06:30Z

This fixes #3690

Modified travis.yml and documentation accordingly.
Modified some doctests in order to run with Python 3.6 (mainly sets, bytes, unicode, etc)

Tests added / passed
Passes flake8 dask

… on travis dask#3690

mrocklin

This is fantastic. Thank you for solving this @eric-bonfadini . I have one small comment, but generally this looks great to me. Thank you so much.

mrocklin · 2018-07-10T21:13:06Z

+    >>> exp_res = {0: slice(-2, -8, -3), 1: slice(-1, -21, -3), 2: slice(-3, -21, -3),
+    ...            3: slice(-2, -21, -3), 4: slice(-1, -21, -3)}
+    >>> _slice_1d(100, [20, 20, 20, 20, 20], slice(100, 12, -3)) == exp_res
+    True


Would it be possible to include full results here in the output rather than explicitly test? In Python 3.6 we can probably depend on deterministic dict ordering. We could probably also use the # doctest: +NORMALIZE_WHITESPACE pragma to help improve the styling, perhaps something like the following?

>>> _slice_1d(100, [20, 20, 20, 20, 20], slice(100, 12, -3)) # doctest: +NORMALIZE_WHITESPACE {0: slice(-2, -8, -3), 1: slice(-1, -21, -3), 2: slice(-3, -21, -3), 3: slice(-2, -21, -3), 4: slice(-1, -21, -3)}

I originally used the # doctest: +NORMALIZE_WHITESPACE pragma, but in the end I stuck to the doctests documentation workaround: https://docs.python.org/3.6/library/doctest.html#warnings

I agree that with full results and pragma we can improve readability and styling, I will have a look and I will modify this part.

mrocklin · 2018-07-11T11:59:28Z

Looks good!

It seems that there are a couple of small dictionary ordering issues. You can see them here: https://travis-ci.org/dask/dask/jobs/402509642

_____________________________ [doctest] dask.local _____________________________
057 5.  waiting_data: available data to yet-to-be-run-tasks :: {key: {keys}}
058     Real-time equivalent of dependents
059 
060 
061 Examples
062 --------
063 
064 >>> import pprint
065 >>> dsk = {'x': 1, 'y': 2, 'z': (inc, 'x'), 'w': (add, 'z', 'y')}
066 >>> pprint.pprint(start_state_from_dask(dsk)) # doctest: +NORMALIZE_WHITESPACE
Differences (unified diff with -expected +actual):
    @@ -1,4 +1,4 @@
     {'cache': {'x': 1, 'y': 2},
    - 'dependencies': {'w': {'z', 'y'}, 'x': set(), 'y': set(), 'z': {'x'}},
    + 'dependencies': {'w': {'y', 'z'}, 'x': set(), 'y': set(), 'z': {'x'}},
      'dependents': {'w': set(), 'x': {'z'}, 'y': {'w'}, 'z': {'w'}},
      'finished': set(),
/home/travis/build/dask/dask/dask/local.py:66: DocTestFailure
__________________ [doctest] dask.local.start_state_from_dask __________________
Differences (unified diff with -expected +actual):
    @@ -1,4 +1,4 @@
     {'cache': {'x': 1, 'y': 2},
    - 'dependencies': {'w': {'z', 'y'}, 'x': set(), 'y': set(), 'z': {'x'}},
    + 'dependencies': {'w': {'y', 'z'}, 'x': set(), 'y': set(), 'z': {'x'}},
      'dependents': {'w': set(), 'x': {'z'}, 'y': {'w'}, 'z': {'w'}},
      'finished': set(),
/home/travis/build/dask/dask/dask/local.py:157: DocTestFailure

eric-bonfadini · 2018-07-11T21:23:20Z

This happens because pprint just sorts dicts entries, while sets are printed with a standard repr and not sorted.
You can double check by running a few times this simple script, sometimes you will get "{'y', 'z'}", sometime "{'z', 'y'}":
python3 -c "from pprint import pprint; pprint({'dependencies': {'w': {'y', 'z'}}})"

I didn't notice locally because I had a export PYTHONHASHSEED=42 in my local env that was hiding the issue (in the travis run for non parallel builds PYTHONHASHSEED is not set).
Moreover, in the previous build (https://travis-ci.org/dask/dask/jobs/402406831) that test run correctly.

In order to fix this issue we have some options:
1 - compare the function result with the expected value, like explained in https://docs.python.org/3.6/library/doctest.html#warnings
2 - sort the function result before running the test
3 - set the PYTHONHASHSEED even in non parallel builds

Do you have a preferred option?
Or do you have a better one I didn't think of?

mrocklin · 2018-07-11T22:17:45Z

I suggest that we just skip the doctest. The documentation value is probably greater here than the testing value, which is very likely covered elsewhere.

…

On Wed, Jul 11, 2018 at 4:23 PM, Eric Bonfadini ***@***.***> wrote: This happens because pprint just sorts dicts entries, while sets are printed with a standard repr and not sorted. You can double check by running a few times this simple script, sometimes you will get "{'y', 'z'}", sometime "{'z', 'y'}": python3 -c "from pprint import pprint; pprint({'dependencies': {'w': {'y', 'z'}}})" I didn't notice locally because I had a export PYTHONHASHSEED=42 in my local env that was hiding the issue (in the travis run for non parallel builds PYTHONHASHSEED is not set). Moreover, in the previous build (https://travis-ci.org/dask/ dask/jobs/402406831) that test run correctly. In order to fix this issue we have some options: 1 - compare the function result with the expected value, like explained in https://docs.python.org/3.6/library/doctest.html#warnings 2 - sort the function result before running the test 3 - set the PYTHONHASHSEED even in non parallel builds Do you have a preferred option? Or do you have a better one I didn't think of? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#3744 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AASszLZdGCreYPE8eIGoypPBnpXX15YGks5uFmzKgaJpZM4VKH47> .

mrocklin · 2018-07-11T22:17:59Z

# doctest: +SKIP On Wed, Jul 11, 2018 at 5:17 PM, Matthew Rocklin <[email protected]> wrote:

…

I suggest that we just skip the doctest. The documentation value is probably greater here than the testing value, which is very likely covered elsewhere. On Wed, Jul 11, 2018 at 4:23 PM, Eric Bonfadini ***@***.***> wrote: > This happens because pprint just sorts dicts entries, while sets are > printed with a standard repr and not sorted. > You can double check by running a few times this simple script, sometimes > you will get "{'y', 'z'}", sometime "{'z', 'y'}": > python3 -c "from pprint import pprint; pprint({'dependencies': {'w': > {'y', 'z'}}})" > > I didn't notice locally because I had a export PYTHONHASHSEED=42 in my > local env that was hiding the issue (in the travis run for non parallel > builds PYTHONHASHSEED is not set). > Moreover, in the previous build (https://travis-ci.org/dask/da > sk/jobs/402406831) that test run correctly. > > In order to fix this issue we have some options: > 1 - compare the function result with the expected value, like explained > in https://docs.python.org/3.6/library/doctest.html#warnings > 2 - sort the function result before running the test > 3 - set the PYTHONHASHSEED even in non parallel builds > > Do you have a preferred option? > Or do you have a better one I didn't think of? > > — > You are receiving this because you commented. > Reply to this email directly, view it on GitHub > <#3744 (comment)>, or mute > the thread > <https://github.com/notifications/unsubscribe-auth/AASszLZdGCreYPE8eIGoypPBnpXX15YGks5uFmzKgaJpZM4VKH47> > . >

…#3690

mrocklin · 2018-07-12T12:04:42Z

This looks great! Thanks @eric-bonfadini ! Merging.

eric-bonfadini added 2 commits July 10, 2018 22:49

Modified coverage and docs in order to execute doctests in Python 3.6…

2c89f86

… on travis dask#3690

Modified doctests in order to run with Python 3.6 dask#3690

d3d5bf9

mrocklin reviewed Jul 10, 2018

View reviewed changes

Added NORMALIZE_WHITESPACE pragma in order to improve styling dask#3690

0f0b91a

Added doctest: +SKIP directive in order to avoid ordering issues dask…

9e0c054

…#3690

mrocklin merged commit d8f7ad3 into dask:master Jul 12, 2018

jakirkham mentioned this pull request Jul 20, 2018

Add the @ operator to the delayed objects #3691

Merged

2 tasks

eric-bonfadini deleted the master_doctest-py3_3690 branch October 9, 2024 10:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Change doctests to Python 3 #3690#3744

Change doctests to Python 3 #3690#3744
mrocklin merged 4 commits into
dask:masterfrom
eric-bonfadini:master_doctest-py3_3690

eric-bonfadini commented Jul 10, 2018 •

edited

Loading

Uh oh!

mrocklin left a comment

Uh oh!

mrocklin Jul 10, 2018

Uh oh!

eric-bonfadini Jul 10, 2018

Uh oh!

mrocklin commented Jul 11, 2018

Uh oh!

eric-bonfadini commented Jul 11, 2018

Uh oh!

mrocklin commented Jul 11, 2018 via email

Uh oh!

mrocklin commented Jul 11, 2018 via email

Uh oh!

mrocklin commented Jul 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

eric-bonfadini commented Jul 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mrocklin left a comment

Choose a reason for hiding this comment

Uh oh!

mrocklin Jul 10, 2018

Choose a reason for hiding this comment

Uh oh!

eric-bonfadini Jul 10, 2018

Choose a reason for hiding this comment

Uh oh!

mrocklin commented Jul 11, 2018

Uh oh!

eric-bonfadini commented Jul 11, 2018

Uh oh!

mrocklin commented Jul 11, 2018 via email

Uh oh!

mrocklin commented Jul 11, 2018 via email

Uh oh!

mrocklin commented Jul 12, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

eric-bonfadini commented Jul 10, 2018 •

edited

Loading