You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/10/20 20:53:06 UTC
[GitHub] [commons-math] arturobernalg opened a new pull request, #217: java 8 improvements.
arturobernalg opened a new pull request, #217:
URL: https://github.com/apache/commons-math/pull/217
* Replaced Anonymous type with lambda
* Replace Collections.sort() with List.sort()
* Replace Lambda with method reference
* Simplifiable 'Map' operations
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-math] aherbert commented on pull request #217: java 8 improvements.
Posted by GitBox <gi...@apache.org>.
aherbert commented on PR #217:
URL: https://github.com/apache/commons-math/pull/217#issuecomment-1286172115
Still failing on the same checkstyle violation. Run locally: `mvn checkstyle:check` and fix the neuralnetworks module (and maybe others).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-math] arturobernalg closed pull request #217: java 8 improvements.
Posted by "arturobernalg (via GitHub)" <gi...@apache.org>.
arturobernalg closed pull request #217: java 8 improvements.
URL: https://github.com/apache/commons-math/pull/217
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-math] aherbert commented on a diff in pull request #217: java 8 improvements.
Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #217:
URL: https://github.com/apache/commons-math/pull/217#discussion_r1002012928
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/differentiation/SparseGradient.java:
##########
@@ -382,7 +360,7 @@ public SparseGradient signum() {
public SparseGradient copySign(final SparseGradient sign) {
final long m = Double.doubleToLongBits(value);
final long s = Double.doubleToLongBits(sign.value);
- if ((m >= 0 && s >= 0) || (m < 0 && s < 0)) { // Sign is currently OK
Review Comment:
Didn't notice this before. I would not remove the extra parentheses as it makes it obvious over knowing the operator precedence of && over ||.
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/fitting/SimpleCurveFitter.java:
##########
@@ -318,8 +317,8 @@ private WeightedObservedPoint[] getInterpolationPointsForY(WeightedObservedPoint
private boolean isBetween(double value,
double boundary1,
double boundary2) {
- return (value >= boundary1 && value <= boundary2) ||
Review Comment:
Please revert
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/differentiation/SparseGradient.java:
##########
@@ -393,7 +371,7 @@ public SparseGradient copySign(final SparseGradient sign) {
public SparseGradient copySign(final double sign) {
final long m = Double.doubleToLongBits(value);
final long s = Double.doubleToLongBits(sign);
- if ((m >= 0 && s >= 0) || (m < 0 && s < 0)) { // Sign is currently OK
Review Comment:
Again please revert
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/ode/nonstiff/AdamsNordsieckFieldTransformer.java:
##########
@@ -170,9 +170,9 @@ private AdamsNordsieckFieldTransformer(final Field<T> field, final int n) {
// Nordsieck to multistep, then shifting rows to represent step advance
// then applying inverse transform
T[][] shiftedP = bigP.getData();
- for (int i = shiftedP.length - 1; i > 0; --i) {
- // shift rows
- shiftedP[i] = shiftedP[i - 1];
+ // shift rows
+ if (shiftedP.length - 1 >= 0) {
Review Comment:
use `> 0`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-math] aherbert commented on a diff in pull request #217: java 8 improvements.
Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #217:
URL: https://github.com/apache/commons-math/pull/217#discussion_r1001111243
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/ode/nonstiff/AdamsNordsieckFieldTransformer.java:
##########
@@ -170,10 +170,8 @@ private AdamsNordsieckFieldTransformer(final Field<T> field, final int n) {
// Nordsieck to multistep, then shifting rows to represent step advance
// then applying inverse transform
T[][] shiftedP = bigP.getData();
- for (int i = shiftedP.length - 1; i > 0; --i) {
- // shift rows
- shiftedP[i] = shiftedP[i - 1];
- }
+ // shift rows
+ if (shiftedP.length - 1 >= 0) System.arraycopy(shiftedP, 0, shiftedP, 1, shiftedP.length - 1);
Review Comment:
Should this be `if (shiftedP.length -1 > 0)` (since we cannot copy length=0).
Please add enclosing braces.
##########
commons-math-transform/src/main/java/org/apache/commons/math4/transform/FastCosineTransform.java:
##########
@@ -181,7 +181,7 @@ private UnaryOperator<double[]> create(final Norm normalization,
} else {
return normalization == Norm.ORTHO ?
f -> TransformUtils.scaleInPlace(fct(f), Math.sqrt(2d / (f.length - 1))) :
- f -> fct(f);
+ this::fct;
Review Comment:
Whitespace indentation
##########
commons-math-transform/src/main/java/org/apache/commons/math4/transform/FastSineTransform.java:
##########
@@ -188,7 +188,7 @@ private UnaryOperator<double[]> create(final Norm normalization,
} else {
return normalization == Norm.ORTHO ?
f -> TransformUtils.scaleInPlace(fst(f), Math.sqrt(2d / f.length)) :
- f -> fst(f);
+ this::fst;
Review Comment:
Indentation again
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-math] aherbert commented on a diff in pull request #217: java 8 improvements.
Posted by GitBox <gi...@apache.org>.
aherbert commented on code in PR #217:
URL: https://github.com/apache/commons-math/pull/217#discussion_r1002382575
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/ml/clustering/ClusterEvaluator.java:
##########
@@ -50,6 +50,6 @@ public interface ClusterEvaluator {
static <T extends Clusterable> ClusterRanking ranking(ClusterEvaluator eval) {
return eval.isBetterScore(1, 2) ?
clusters -> 1 / eval.score(clusters) :
- clusters -> eval.score(clusters);
+ eval::score;
Review Comment:
This has incorrect indentation
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/fitting/SimpleCurveFitter.java:
##########
@@ -319,7 +318,7 @@ private boolean isBetween(double value,
double boundary1,
double boundary2) {
return (value >= boundary1 && value <= boundary2) ||
- (value >= boundary2 && value <= boundary1);
+ (value >= boundary2 && value <= boundary1);
Review Comment:
Please revert the indentation too
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-math] arturobernalg commented on a diff in pull request #217: java 8 improvements.
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on code in PR #217:
URL: https://github.com/apache/commons-math/pull/217#discussion_r1002127274
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/ode/nonstiff/AdamsNordsieckFieldTransformer.java:
##########
@@ -170,9 +170,9 @@ private AdamsNordsieckFieldTransformer(final Field<T> field, final int n) {
// Nordsieck to multistep, then shifting rows to represent step advance
// then applying inverse transform
T[][] shiftedP = bigP.getData();
- for (int i = shiftedP.length - 1; i > 0; --i) {
- // shift rows
- shiftedP[i] = shiftedP[i - 1];
+ // shift rows
+ if (shiftedP.length - 1 >= 0) {
Review Comment:
done.
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/fitting/SimpleCurveFitter.java:
##########
@@ -318,8 +317,8 @@ private WeightedObservedPoint[] getInterpolationPointsForY(WeightedObservedPoint
private boolean isBetween(double value,
double boundary1,
double boundary2) {
- return (value >= boundary1 && value <= boundary2) ||
Review Comment:
done.
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/differentiation/SparseGradient.java:
##########
@@ -393,7 +371,7 @@ public SparseGradient copySign(final SparseGradient sign) {
public SparseGradient copySign(final double sign) {
final long m = Double.doubleToLongBits(value);
final long s = Double.doubleToLongBits(sign);
- if ((m >= 0 && s >= 0) || (m < 0 && s < 0)) { // Sign is currently OK
Review Comment:
done.
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/analysis/differentiation/SparseGradient.java:
##########
@@ -382,7 +360,7 @@ public SparseGradient signum() {
public SparseGradient copySign(final SparseGradient sign) {
final long m = Double.doubleToLongBits(value);
final long s = Double.doubleToLongBits(sign.value);
- if ((m >= 0 && s >= 0) || (m < 0 && s < 0)) { // Sign is currently OK
Review Comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-math] arturobernalg commented on a diff in pull request #217: java 8 improvements.
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on code in PR #217:
URL: https://github.com/apache/commons-math/pull/217#discussion_r1002560605
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/ml/clustering/ClusterEvaluator.java:
##########
@@ -50,6 +50,6 @@ public interface ClusterEvaluator {
static <T extends Clusterable> ClusterRanking ranking(ClusterEvaluator eval) {
return eval.isBetterScore(1, 2) ?
clusters -> 1 / eval.score(clusters) :
- clusters -> eval.score(clusters);
+ eval::score;
Review Comment:
Done
##########
commons-math-legacy/src/main/java/org/apache/commons/math4/legacy/fitting/SimpleCurveFitter.java:
##########
@@ -319,7 +318,7 @@ private boolean isBetween(double value,
double boundary1,
double boundary2) {
return (value >= boundary1 && value <= boundary2) ||
- (value >= boundary2 && value <= boundary1);
+ (value >= boundary2 && value <= boundary1);
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-math] arturobernalg commented on pull request #217: java 8 improvements.
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on PR #217:
URL: https://github.com/apache/commons-math/pull/217#issuecomment-1287193161
> Still failing on the same checkstyle violation. Run locally: `mvn checkstyle:check` and fix the neuralnetworks module (and maybe others).
hi @aherbert
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-math] arturobernalg commented on pull request #217: java 8 improvements.
Posted by GitBox <gi...@apache.org>.
arturobernalg commented on PR #217:
URL: https://github.com/apache/commons-math/pull/217#issuecomment-1286164442
done. @aherbert
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-math] codecov-commenter commented on pull request #217: java 8 improvements.
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #217:
URL: https://github.com/apache/commons-math/pull/217#issuecomment-1286140950
# [Codecov](https://codecov.io/gh/apache/commons-math/pull/217?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#217](https://codecov.io/gh/apache/commons-math/pull/217?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f92ac36) into [master](https://codecov.io/gh/apache/commons-math/commit/ec7eb8b32d9dff80f98c506a88c521a0ca3ea6a6?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ec7eb8b) will **increase** coverage by `0.03%`.
> The diff coverage is `96.29%`.
```diff
@@ Coverage Diff @@
## master #217 +/- ##
============================================
+ Coverage 86.55% 86.58% +0.03%
+ Complexity 9810 9802 -8
============================================
Files 533 533
Lines 35579 35558 -21
Branches 6212 6207 -5
============================================
- Hits 30796 30789 -7
+ Misses 3524 3502 -22
- Partials 1259 1267 +8
```
| [Impacted Files](https://codecov.io/gh/apache/commons-math/pull/217?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...s/math4/legacy/ml/clustering/ClusterEvaluator.java](https://codecov.io/gh/apache/commons-math/pull/217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1tYXRoLWxlZ2FjeS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY29tbW9ucy9tYXRoNC9sZWdhY3kvbWwvY2x1c3RlcmluZy9DbHVzdGVyRXZhbHVhdG9yLmphdmE=) | `50.00% <ø> (+16.66%)` | :arrow_up: |
| [...e/commons/math4/transform/FastCosineTransform.java](https://codecov.io/gh/apache/commons-math/pull/217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1tYXRoLXRyYW5zZm9ybS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY29tbW9ucy9tYXRoNC90cmFuc2Zvcm0vRmFzdENvc2luZVRyYW5zZm9ybS5qYXZh) | `100.00% <ø> (ø)` | |
| [...che/commons/math4/transform/FastSineTransform.java](https://codecov.io/gh/apache/commons-math/pull/217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1tYXRoLXRyYW5zZm9ybS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY29tbW9ucy9tYXRoNC90cmFuc2Zvcm0vRmFzdFNpbmVUcmFuc2Zvcm0uamF2YQ==) | `100.00% <ø> (ø)` | |
| [...y/ode/nonstiff/AdamsNordsieckFieldTransformer.java](https://codecov.io/gh/apache/commons-math/pull/217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1tYXRoLWxlZ2FjeS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY29tbW9ucy9tYXRoNC9sZWdhY3kvb2RlL25vbnN0aWZmL0FkYW1zTm9yZHNpZWNrRmllbGRUcmFuc2Zvcm1lci5qYXZh) | `98.59% <50.00%> (-1.41%)` | :arrow_down: |
| [...egacy/analysis/differentiation/SparseGradient.java](https://codecov.io/gh/apache/commons-math/pull/217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1tYXRoLWxlZ2FjeS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY29tbW9ucy9tYXRoNC9sZWdhY3kvYW5hbHlzaXMvZGlmZmVyZW50aWF0aW9uL1NwYXJzZUdyYWRpZW50LmphdmE=) | `96.88% <100.00%> (+0.57%)` | :arrow_up: |
| [...ons/math4/legacy/analysis/solvers/BrentSolver.java](https://codecov.io/gh/apache/commons-math/pull/217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1tYXRoLWxlZ2FjeS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY29tbW9ucy9tYXRoNC9sZWdhY3kvYW5hbHlzaXMvc29sdmVycy9CcmVudFNvbHZlci5qYXZh) | `77.27% <100.00%> (ø)` | |
| [...ommons/math4/legacy/fitting/SimpleCurveFitter.java](https://codecov.io/gh/apache/commons-math/pull/217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1tYXRoLWxlZ2FjeS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY29tbW9ucy9tYXRoNC9sZWdhY3kvZml0dGluZy9TaW1wbGVDdXJ2ZUZpdHRlci5qYXZh) | `74.07% <100.00%> (ø)` | |
| [...pache/commons/math4/legacy/genetics/RandomKey.java](https://codecov.io/gh/apache/commons-math/pull/217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1tYXRoLWxlZ2FjeS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY29tbW9ucy9tYXRoNC9sZWdhY3kvZ2VuZXRpY3MvUmFuZG9tS2V5LmphdmE=) | `89.55% <100.00%> (ø)` | |
| [...linear/scalar/MultiStartMultivariateOptimizer.java](https://codecov.io/gh/apache/commons-math/pull/217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1tYXRoLWxlZ2FjeS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY29tbW9ucy9tYXRoNC9sZWdhY3kvb3B0aW0vbm9ubGluZWFyL3NjYWxhci9NdWx0aVN0YXJ0TXVsdGl2YXJpYXRlT3B0aW1pemVyLmphdmE=) | `73.68% <100.00%> (ø)` | |
| [...linear/scalar/noderiv/HedarFukushimaTransform.java](https://codecov.io/gh/apache/commons-math/pull/217/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Y29tbW9ucy1tYXRoLWxlZ2FjeS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvY29tbW9ucy9tYXRoNC9sZWdhY3kvb3B0aW0vbm9ubGluZWFyL3NjYWxhci9ub2Rlcml2L0hlZGFyRnVrdXNoaW1hVHJhbnNmb3JtLmphdmE=) | `69.76% <100.00%> (ø)` | |
| ... and [10 more](https://codecov.io/gh/apache/commons-math/pull/217/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org