Skip to content

Change doctests to Python 3 #3690#3744

Merged
mrocklin merged 4 commits intodask:masterfrom
eric-bonfadini:master_doctest-py3_3690
Jul 12, 2018
Merged

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

Conversation

@eric-bonfadini
Copy link
Contributor

@eric-bonfadini eric-bonfadini commented Jul 10, 2018

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

Copy link
Member

@mrocklin mrocklin left a comment

Choose a reason for hiding this comment

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

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.

>>> 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
Copy link
Member

Choose a reason for hiding this comment

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

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)}


Copy link
Contributor Author

Choose a reason for hiding this comment

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

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

mrocklin commented Jul 11, 2018 via email

@mrocklin
Copy link
Member

mrocklin commented Jul 11, 2018 via email

@mrocklin
Copy link
Member

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

@mrocklin mrocklin merged commit d8f7ad3 into dask:master Jul 12, 2018
@eric-bonfadini eric-bonfadini deleted the master_doctest-py3_3690 branch October 9, 2024 10:19
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.

Change doctests to Python 3

2 participants