You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tinkerpop.apache.org by dkuppitz <gi...@git.apache.org> on 2015/10/21 18:25:40 UTC
[GitHub] incubator-tinkerpop pull request: Tinkerpop3 861 :: Solve "The Num...
GitHub user dkuppitz opened a pull request:
https://github.com/apache/incubator-tinkerpop/pull/116
Tinkerpop3 861 :: Solve "The Number Problem" for Operator (and follow on operators)
I implemented `NumberHelper`, which a) determines the highest common `Number` type for a list of given `Number`s and b) provides simple operations like `min`, `max`, `add`, `sub`, etc..
The `NumberHelper` is now used in all the `Operator` enums and ensures that the result type is always the highest common `Number` type and thus results are always accurate.
**Before**
```
Operator.mul.apply(1, 0.5d) ==> 0
Operator.add.apply(1, 0.5d) ==> 1
```
**After**
```
Operator.mul.apply(1, 0.5d) ==> 0.5d
Operator.add.apply(1, 0.5d) ==> 1.5d
```
Furthermore `Operator` now works for any `Number` type (e.g. `BigInteger`, `BigDecimal` and any custom `Number` implementation).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/apache/incubator-tinkerpop TINKERPOP3-861
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-tinkerpop/pull/116.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #116
----
commit 66a765e593985edc96268115b3d0bc2450c65b90
Author: Daniel Kuppitz <da...@hotmail.com>
Date: 2015-10-21T00:02:03Z
added mixed type tests (failing for now)
commit e25af2990b387cf191c1fa3f9cb12a2f4318d5a7
Author: Daniel Kuppitz <da...@hotmail.com>
Date: 2015-10-21T00:55:13Z
implemented NumberHelper and fixed Number handling in Operator (also added a few more tests for BigInteger ad BigDecimal)
commit 209ef1a4cb6757d85fa44253ae83dbfd6e2098f4
Author: Daniel Kuppitz <da...@hotmail.com>
Date: 2015-10-21T00:56:15Z
Merge branch 'master' into TINKERPOP3-861
commit 78a0e3d58da45fe5c836845a9e643240f24d369f
Author: Daniel Kuppitz <da...@hotmail.com>
Date: 2015-10-21T01:40:55Z
verified, that Operator/NumberHelper can handle any type that extends the base class Number
commit 47363c88716a40ddc50ccd9df520514c8d2903ca
Author: Daniel Kuppitz <da...@hotmail.com>
Date: 2015-10-21T11:27:16Z
Merge branch 'master' into TINKERPOP3-861
commit af1f2a82d4b6c2f8bff61a98a1efe01b43499697
Author: Daniel Kuppitz <da...@hotmail.com>
Date: 2015-10-21T16:12:16Z
fixed number types in (Groovy)SackTest
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-861: Solve "The Numbe...
Posted by dkuppitz <gi...@git.apache.org>.
Github user dkuppitz commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/116#issuecomment-150290829
@spmallette I updated CHANGELOG and the upgrade docs.
Also, since forgot it in my initial comment:
My vote: +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-861: Solve "The Numbe...
Posted by pluradj <gi...@git.apache.org>.
Github user pluradj commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/116#issuecomment-150294697
Built cleanly with `mvn clean install`.
Ran a few manual tests against Operator from the Gremlin Console.
Reviewed the doc updates.
Learned something new about TinkerPop today.
Vote: +1
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-861: Solve "The Numbe...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-tinkerpop/pull/116
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-861: Solve "The Numbe...
Posted by okram <gi...@git.apache.org>.
Github user okram commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/116#issuecomment-149957481
This is really solid work. The code is clean, the test cases are thorough.
* I am assuming you ran `mvn clean install`.
* Be sure to update CHANGELOG on merge.
* Are there other areas of the codebase that can leverage `NumberHelper`? (perhaps email dev@ and lets discuss there after merge).
* This is potentially *breaking* if someone was relying on 0.5 * 1 to be 0. :) ... Regardless add this to the upgrade docs too so people know.
VOTE +1.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-tinkerpop pull request: TINKERPOP3-861: Solve "The Numbe...
Posted by spmallette <gi...@git.apache.org>.
Github user spmallette commented on the pull request:
https://github.com/apache/incubator-tinkerpop/pull/116#issuecomment-150205625
@dkuppitz do you mind amending the PR to include the upgrade docs? @okram raises a good point on that - would like to see that in place prior to voting if that's cool.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---