Skip to content

Conversation

@HyukjinKwon
Copy link
Member

What changes were proposed in this pull request?

This PR proposes to actually run the doctests in ml/image.py.

How was this patch tested?

doctests in python/pyspark/ml/image.py.

@HyukjinKwon
Copy link
Member Author

cc @MrBago and @imatiach-msft, could you take a look when you are available please?

>>> df = ImageSchema.readImages('data/mllib/images/kittens', recursive=True)
>>> df.count()
4
5
Copy link
Member Author

Choose a reason for hiding this comment

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

It becomes 5 since we don't dropImageFailures.

Copy link
Contributor

Choose a reason for hiding this comment

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

nice catch!

@SparkQA
Copy link

SparkQA commented Jan 17, 2018

Test build #86268 has finished for PR 20294 at commit 9912444.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@imatiach-msft
Copy link
Contributor

nice fix! Great to see the doctests run.

Copy link
Member

@BryanCutler BryanCutler left a comment

Choose a reason for hiding this comment

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

LGTM

asfgit pushed a commit that referenced this pull request Jan 17, 2018
## What changes were proposed in this pull request?

This PR proposes to actually run the doctests in `ml/image.py`.

## How was this patch tested?

doctests in `python/pyspark/ml/image.py`.

Author: hyukjinkwon <[email protected]>

Closes #20294 from HyukjinKwon/trigger-image.

(cherry picked from commit 45ad97d)
Signed-off-by: hyukjinkwon <[email protected]>
@HyukjinKwon
Copy link
Member Author

Merged to master and branch-2.3.

Thanks for reviewing this, @srowen, @imatiach-msft, @MrBago and @BryanCutler.

@asfgit asfgit closed this in 45ad97d Jan 17, 2018
@HyukjinKwon HyukjinKwon deleted the trigger-image branch October 16, 2018 12:44
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.

5 participants