Skip to content

Remove skips from doctests (4 of 6)#7865

Merged
jsignell merged 2 commits intodask:mainfrom
zzhengnan:enable-doctest-for-bag
Jul 28, 2021
Merged

Remove skips from doctests (4 of 6)#7865
jsignell merged 2 commits intodask:mainfrom
zzhengnan:enable-doctest-for-bag

Conversation

@zzhengnan
Copy link
Copy Markdown
Contributor

@zzhengnan zzhengnan commented Jul 3, 2021

Comment on lines 1938 to +1943
When what you really wanted was more along the lines of the following:

>>> list(fizzbuzzz) # doctest: +SKIP
[(0, 0), (3, None), (None, 5), (6, None), (None 10), (9, None),
(12, None), (15, 15), (18, None), (None, 20), (None, 25), (None, 30)]
>>> list(fizzbuzz) # doctest: +SKIP
(0, 0), (3, None), (None, 5), (6, None), (9, None), (None, 10),
(12, None), (15, 15), (18, None), (None, 20),
(21, None), (24, None), (None, 25), (27, None), (30, 30)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Re the following comment, it's not clear to me what the original author was going for, so I took my best guess.

When what you really wanted was more along the lines of the following

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

huh yeah. I am not sure either. I guess what you have is fine but just wrap it in square brackets like the original.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Will do.

Only problem is I'd accidentally deleted my fork so can't push changes to the feature branch. As seen below, the feature branch is grayed out and can no longer be clicked on (normally clicking on it takes you to the fork). This applies to #7864 as well.
image

Any idea what I can do here without having to open a new PR? I do still have access to the fork locally so no work is lost but I'd prefer not to open a new PR if possible.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh wierd. I am going to see if I can still push.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nope.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm just going to merge this since the last comment is about a skipped doctest output.

Comment on lines -24 to +27
>>> import dask.bag as db # doctest: +SKIP
... from dask.bag import random
...
... b = db.from_sequence(range(5), npartitions=2)
... list(random.sample(b, 3).compute())
>>> import dask.bag as db
>>> from dask.bag import random
>>> b = db.from_sequence(range(5), npartitions=2)
>>> list(random.sample(b, 3).compute()) # doctest: +SKIP
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Moving the SKIP to the actual line that we want to skip feels more intuitive in my opinion.

@zzhengnan zzhengnan changed the title Remove skips from doctests (part 3) Remove skips from doctests (4 of 6) Jul 5, 2021
@zzhengnan
Copy link
Copy Markdown
Contributor Author

@jsignell Ready for review.

@jsignell jsignell merged commit 69502ff into dask:main Jul 28, 2021
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.

2 participants