Skip to content

[SQL] Using safe floating-point numbers in doctest #1925

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
liancheng wants to merge 2 commits into from
Closed

[SQL] Using safe floating-point numbers in doctest #1925

liancheng wants to merge 2 commits into from

Conversation

@liancheng
Copy link
Contributor

@liancheng liancheng commented Aug 13, 2014

Test code in sql.py tries to compare two floating-point numbers directly, and cased build failure(s).

Doctest documentation recommends using numbers in the form of I/2**J to avoid the precision issue.

@SparkQA
Copy link

SparkQA commented Aug 13, 2014

QA tests have started for PR 1925. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18443/consoleFull

@liancheng
Copy link
Contributor Author

liancheng commented Aug 13, 2014

Closing this since it's already fixed in 882da57. /cc @yhuai

@liancheng liancheng closed this Aug 13, 2014
@liancheng
Copy link
Contributor Author

liancheng commented Aug 13, 2014

After some thoughts, I think we still need to use safer floating-point numbers (I/2**J), since the 2.1... pattern doesn't cover evil cases like 2.099999999999. Reopening this. /cc @davies

@liancheng liancheng reopened this Aug 13, 2014
@SparkQA
Copy link

SparkQA commented Aug 13, 2014

QA tests have started for PR 1925. This patch merges cleanly.
View progress: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18445/consoleFull

@SparkQA
Copy link

SparkQA commented Aug 13, 2014

QA results for PR 1925:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18443/consoleFull

@SparkQA
Copy link

SparkQA commented Aug 13, 2014

QA results for PR 1925:
- This patch PASSES unit tests.
- This patch merges cleanly
- This patch adds no public classes

For more information see test ouptut:
https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18445/consoleFull

davies
davies reviewed Aug 13, 2014
View reviewed changes
python/pyspark/sql.py Outdated
Copy link
Contributor

@davies davies Aug 13, 2014

Choose a reason for hiding this comment

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

The ... is not needed any more

Copy link
Contributor Author

@liancheng liancheng Aug 14, 2014

Choose a reason for hiding this comment

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

Removed ..., thanks :)

@davies
Copy link
Contributor

davies commented Aug 13, 2014

This issue is introduced by convert Float to Double in JVM:

scala> (1.0f + 1.1).toFloat.toDouble
res13: Double = 2.0999999046325684

scala> (1.0f + 1.1f).toDouble
res14: Double = 2.0999999046325684

scala> 1.0 + 1.1f
res16: Double = 2.100000023841858

@marmbrus
Copy link
Contributor

marmbrus commented Aug 14, 2014

@davies so does this solution seem reasonable?

@davies
Copy link
Contributor

davies commented Aug 14, 2014

Yes, look good to me.

@davies
Copy link
Contributor

davies commented Aug 14, 2014

scala> 1.5f.toDouble
res1: Double = 1.5

scala> 2.5f.toDouble
res2: Double = 2.5

@marmbrus
Copy link
Contributor

marmbrus commented Aug 16, 2014

Thanks, I've merged this into master and 1.1

@asfgit asfgit closed this in b4a0592 Aug 16, 2014
asfgit pushed a commit that referenced this pull request Aug 16, 2014
@liancheng @marmbrus
[SQL] Using safe floating-point numbers in doctest
8c79574 
Test code in `sql.py` tries to compare two floating-point numbers directly, and cased [build failure(s)](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18365/consoleFull).

[Doctest documentation](https://docs.python.org/3/library/doctest.html#warnings) recommends using numbers in the form of `I/2**J` to avoid the precision issue.

Author: Cheng Lian <[email protected]>

Closes #1925 from liancheng/fix-pysql-fp-test and squashes the following commits:

0fbf584 [Cheng Lian] Removed unnecessary `...' from inferSchema doctest
e8059d4 [Cheng Lian] Using safe floating-point numbers in doctest

(cherry picked from commit b4a0592)
Signed-off-by: Michael Armbrust <[email protected]>
@liancheng liancheng deleted the fix-pysql-fp-test branch August 18, 2014 02:55
xiliu82 pushed a commit to xiliu82/spark that referenced this pull request Sep 4, 2014
@liancheng @conviva-zz
[SQL] Using safe floating-point numbers in doctest
7f5bf16 
Test code in `sql.py` tries to compare two floating-point numbers directly, and cased [build failure(s)](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/18365/consoleFull).

[Doctest documentation](https://docs.python.org/3/library/doctest.html#warnings) recommends using numbers in the form of `I/2**J` to avoid the precision issue.

Author: Cheng Lian <[email protected]>

Closes apache#1925 from liancheng/fix-pysql-fp-test and squashes the following commits:

0fbf584 [Cheng Lian] Removed unnecessary `...' from inferSchema doctest
e8059d4 [Cheng Lian] Using safe floating-point numbers in doctest
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@liancheng @SparkQA @davies @marmbrus