[jira] [Comment Edited] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[ https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114581#comment-16114581 ] Karl Richter edited comment on MATH-1426 at 8/4/17 4:25 PM: bq. So, in this issue's case, using a randomly populated array is just a convenience: the code could be checked with a "manually" chosen set of values; hence, we can choose any seed (and keep that one forever). Interesting, thanks for the detailed explanation. I imitated the code of other tests as suggested. bq. For "Commons Math" main code (in the "main" directory), only the API part would become a dependency. bq. I also think that "slf4j-api" is a safe choice (but this must be decided on the ML). One discussion is http://markmail.org/message/pf46s7775bt37swk for reference. That's tricky because of the wide range of interests. SLF4J is the most often declared dependency in github.com projectshttps://en.wikipedia.org/wiki/SLF4J. bq. I'm no maven expert; please raise this issue on the "dev" ML (use different posts for different subjects). Done. The title is `[MATH] Enforce run of checkstyle-maven-plugin in validate instead of site phase` bq. I didn't figure out what you mean by wrong tabulation bq. For example, I'd think that in most of the code, the alignment would be (to be viewed with a fixed-width font...): I took over your example, the rest will be covered by checkstyle enforcement. was (Author: krichter): bq. So, in this issue's case, using a randomly populated array is just a convenience: the code could be checked with a "manually" chosen set of values; hence, we can choose any seed (and keep that one forever). Interesting, thanks for the detailed explanation. I imitated the code of other tests as suggested. bq. For "Commons Math" main code (in the "main" directory), only the API part would become a dependency. I also think that "slf4j-api" is a safe choice (but this must be decided on the ML). One discussion is http://markmail.org/message/pf46s7775bt37swk for reference. That's tricky because of the wide range of interests. SLF4J is the most often declared dependency in github.com projectshttps://en.wikipedia.org/wiki/SLF4J. bq. I'm no maven expert; please raise this issue on the "dev" ML (use different posts for different subjects). Done. The title is `[MATH] Enforce run of checkstyle-maven-plugin in validate instead of site phase` bq. I didn't figure out what you mean by wrong tabulation bq. For example, I'd think that in most of the code, the alignment would be (to be viewed with a fixed-width font...): I took over your example, the rest will be covered by checkstyle enforcement. > Add constructor with Double[] argument to DescriptiveStatistics > --- > > Key: MATH-1426 > URL: https://issues.apache.org/jira/browse/MATH-1426 > Project: Commons Math > Issue Type: Improvement >Affects Versions: 4.0 >Reporter: Karl Richter > Fix For: 4.0 > > Attachments: > 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch, > 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > > > It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`. > The patch is available at https://github.com/apache/commons-math/pull/54 in > form of a PR as well. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Updated] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[ https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Karl Richter updated MATH-1426: --- Attachment: (was: 0002-added-constructor-with-Double-argument-to-Descriptiv.patch) > Add constructor with Double[] argument to DescriptiveStatistics > --- > > Key: MATH-1426 > URL: https://issues.apache.org/jira/browse/MATH-1426 > Project: Commons Math > Issue Type: Improvement >Affects Versions: 4.0 >Reporter: Karl Richter > Fix For: 4.0 > > Attachments: > 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch, > 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > > > It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`. > The patch is available at https://github.com/apache/commons-math/pull/54 in > form of a PR as well. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Updated] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[ https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Karl Richter updated MATH-1426: --- Attachment: 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > Add constructor with Double[] argument to DescriptiveStatistics > --- > > Key: MATH-1426 > URL: https://issues.apache.org/jira/browse/MATH-1426 > Project: Commons Math > Issue Type: Improvement >Affects Versions: 4.0 >Reporter: Karl Richter > Fix For: 4.0 > > Attachments: > 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch, > 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > > > It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`. > The patch is available at https://github.com/apache/commons-math/pull/54 in > form of a PR as well. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[ https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114581#comment-16114581 ] Karl Richter commented on MATH-1426: bq. So, in this issue's case, using a randomly populated array is just a convenience: the code could be checked with a "manually" chosen set of values; hence, we can choose any seed (and keep that one forever). Interesting, thanks for the detailed explanation. I imitated the code of other tests as suggested. bq. For "Commons Math" main code (in the "main" directory), only the API part would become a dependency. I also think that "slf4j-api" is a safe choice (but this must be decided on the ML). One discussion is http://markmail.org/message/pf46s7775bt37swk for reference. That's tricky because of the wide range of interests. SLF4J is the most often declared dependency in github.com projectshttps://en.wikipedia.org/wiki/SLF4J. bq. I'm no maven expert; please raise this issue on the "dev" ML (use different posts for different subjects). Done. The title is `[MATH] Enforce run of checkstyle-maven-plugin in validate instead of site phase` bq. I didn't figure out what you mean by wrong tabulation bq. For example, I'd think that in most of the code, the alignment would be (to be viewed with a fixed-width font...): I took over your example, the rest will be covered by checkstyle enforcement. > Add constructor with Double[] argument to DescriptiveStatistics > --- > > Key: MATH-1426 > URL: https://issues.apache.org/jira/browse/MATH-1426 > Project: Commons Math > Issue Type: Improvement >Affects Versions: 4.0 >Reporter: Karl Richter > Fix For: 4.0 > > Attachments: > 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch, > 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > > > It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`. > The patch is available at https://github.com/apache/commons-math/pull/54 in > form of a PR as well. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[
https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114534#comment-16114534
]
Gilles commented on MATH-1426:
--
bq. I don't see the downside of using a different "fixed" \[...\]
See some of the context in the discussion about MATH-1314.
Side-note: the resulting code is now contained in "Commons RNG".
So, in this issue's case, using a randomly populated array is just a
convenience: the code could be checked with a "manually" chosen set of values;
hence, we can choose any seed (and keep that one forever).
Side-note: many tests in "Commons RNG" do use variable/random seeds, for the
reason that some can (and sometimes do) fail because of a "bad" seed.
bq. I'm a huge fan of configurable logging framework \[...\]
I also think that it is very valuable.
There were several discussions (see the "dev" ML archive) about adding logging
statements to "Commons Math".
You are most welcome to bring your opinion to the "dev" ML (if your are willing
to do the work in case the proposal is accepted).
bq. The logging of the generator seed is necessary unless the above is wrong
I hope that my explanation above makes sense.
It aimed at saying that, in this issue's case, a random seed blurs what is
really necessary to validate the code under test.
bq. I suggest slf4j-api + logback-classic
For "Commons Math" main code (in the "main" directory), only the API part would
become a dependency.
I also think that "slf4j-api" is a safe choice (but this must be decided on the
ML).
For the test code, we'd need to choose an implementation: if you insist on
"logback", since this is an Apache project, be prepared to argue against the
obvious choice: [Log4J2|http://logging.apache.org/log4j/2.x/] ;)
bq. I suggest you move `maven-checkstyle-plugin` from `reporting` to `build`
with
I'm no maven expert; please raise this issue on the "dev" ML (use different
posts for different subjects).
bq. That reveals some hundred issues which should either be silenced or fixed
I guess you mean that those are triggered when checking the "test" code. Much
less stringent attention was given to that part.
bq. I didn't figure out what you mean by wrong tabulation
For example, I'd think that in most of the code, the alignment would be (to be
viewed with a fixed-width font...):
{code}
Assert.assertEquals(original.getGeometricMean(),
instance.getGeometricMean(),
0);
{code}
rather than (as in your patch):
{code}
Assert.assertEquals(original.getGeometricMean(),
instance.getGeometricMean(),
0);
{code}
\[But there is indeed a lot of the "test" codes that does not comply. There,
it's just nit-picking but a uniform style for the "main" part is IMHO quite
useful to reduce "noise" while reading the code.\]
> Add constructor with Double[] argument to DescriptiveStatistics
> ---
>
> Key: MATH-1426
> URL: https://issues.apache.org/jira/browse/MATH-1426
> Project: Commons Math
> Issue Type: Improvement
>Affects Versions: 4.0
>Reporter: Karl Richter
> Fix For: 4.0
>
> Attachments:
> 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch,
> 0002-added-constructor-with-Double-argument-to-Descriptiv.patch
>
>
> It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`.
> The patch is available at https://github.com/apache/commons-math/pull/54 in
> form of a PR as well.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (NET-641) SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
[
https://issues.apache.org/jira/browse/NET-641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114477#comment-16114477
]
pin_ptr commented on NET-641:
-
In my opinion, javadoc is wrong.
It does not cause confusion if number of IP addresses that returns true when
passed to isInRange() and number of IP addresses returned from
getAddressCount() is always the same.
(Javadoc says they are differ if isInclusiveHostCount is true)
> SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
>
>
> Key: NET-641
> URL: https://issues.apache.org/jira/browse/NET-641
> Project: Commons Net
> Issue Type: Bug
>Affects Versions: 3.6
> Environment: Windows; JDK8; common-net 3.6
>Reporter: pin_ptr
>Priority: Minor
> Fix For: 3.7
>
>
> Code:
> import org.apache.commons.net.util.SubnetUtils;
> public class A {
> public static void main(String[] args) {
> System.out.println(new
> SubnetUtils("192.168.1.0/30").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/31").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/32").getInfo().isInRange("0.0.0.0"));
> }
> }
> Result:
> false
> true
> true
> Expected:
> false
> false
> false
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Comment Edited] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[
https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114457#comment-16114457
]
Karl Richter edited comment on MATH-1426 at 8/4/17 2:47 PM:
Thanks for your review. You have impressive coding discipline!
> Prefer several small test methods (one for each tested functionality) even if
> some boiler-plate code repetition do occur. Having "testInit1()",
> "testInit2()", ... is fine (but more meaningful names are preferred if
> possible).
Done.
> If using random data, use a fixed seed, unless the behaviour under test has
> intrinsic variability (which is not the case here).
I don't see the downside of using a different "fixed" ("fixed" for the run of
the test, not "fixed" as you mean it) seed for each run of the unit tests, e.g.
based on the test start time millis, and log the seed so that reproduction in
case of test failure is possible since we're working with pseudo-random
generators which can be re-initialized with the same seed to produce the same
pseudo-random results. In case you agree, how do I initialize a Commons RNG
with such a variable seed? If you don't I'll comply with your explanation.
> Make test sets small (as long as they can reasonably check the functionality)
> to avoid long-running "mvn test"; here I don't think that arrays of length
> 1048576 were needed.
Agreed. I chose 1024.
> Comment out debugging output ("System.out.println")
I'm a huge fan of configurable logging framework which on the one hand require
the evaluation of one very cheap statement, but on the other minimize
controversy between devs and avoid adding previously deleted code (since you
can turn logging statements off in your `logback.xml` or whather is used). I
wouldn't speak of "debugging" statements since it's either debugging or
logging. The logging of the generator seed is necessary unless the above is
wrong, I guess you'll shed some light on this issue.
Do you have interest in adding a logging framework (I suggest slf4j-api +
logback-classic). There're about 100 System.out.print statements in the code,
some commented out. I'd provide a patch if you want.
> Apply a uniform coding style (e.g. there must be a space around operators,
> and the tabulation is wrong).
I suggest you move `maven-checkstyle-plugin` from `reporting` to `build` with
{code:java}
checkstyle
check
validate
{code}
That reveals some hundred issues which should either be silenced or fixed
(almost all of them are Javadoc issues which you might deactive for the check
and put on your schedule to fix later). It minimizes communication overhead
before reviewing contributions like in this situation. The issues you're
describing are not revealed by checkstyle and I didn't figure out what you mean
by wrong tabulation - no need to explain if you change the checkstyle, since
then I can rebase the patch.
was (Author: krichter):
Thanks for your review. You have impressive coding discipline!
> Prefer several small test methods (one for each tested functionality) even if
> some boiler-plate code repetition do occur. Having "testInit1()",
> "testInit2()", ... is fine (but more meaningful names are preferred if
> possible).
Done.
> If using random data, use a fixed seed, unless the behaviour under test has
> intrinsic variability (which is not the case here).
I don't see the downside of using a different "fixed" ("fixed" for the run of
the test, not "fixed" as you mean it) seed for each run of the unit tests, e.g.
based on the test start time millis, and log the seed so that reproduction in
case of test failure is possible since we're working with pseudo-random
generators which can be re-initialized with the same seed to produce the same
pseudo-random results. In case you agree, how do I initialize a Commons RNG
with such a variable seed?
> Make test sets small (as long as they can reasonably check the functionality)
> to avoid long-running "mvn test"; here I don't think that arrays of length
> 1048576 were needed.
Agreed. I chose 1024.
> Comment out debugging output ("System.out.println")
I'm a huge fan of configurable logging framework which on the one hand require
the evaluation of one very cheap statement, but on the other minimize
controversy between devs and avoid adding previously deleted code (since you
can turn logging statements off in your `logback.xml` or whather is used). I
wouldn't speak of "debugging" statements since it's either debugging or
logging. The logging of the generator seed is necessary unless the above is
wrong, I guess you'll shed some light on this issue.
Do you have interest in adding a logging framework (I suggest slf4j-api +
logback-classic). There're about 100 System.out.print statements in the code,
some commented out. I'd
[jira] [Updated] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[ https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Karl Richter updated MATH-1426: --- Attachment: (was: 0002-added-constructor-with-Double-argument-to-Descriptiv.patch) > Add constructor with Double[] argument to DescriptiveStatistics > --- > > Key: MATH-1426 > URL: https://issues.apache.org/jira/browse/MATH-1426 > Project: Commons Math > Issue Type: Improvement >Affects Versions: 4.0 >Reporter: Karl Richter > Fix For: 4.0 > > Attachments: > 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch, > 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > > > It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`. > The patch is available at https://github.com/apache/commons-math/pull/54 in > form of a PR as well. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Updated] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[ https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Karl Richter updated MATH-1426: --- Attachment: 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > Add constructor with Double[] argument to DescriptiveStatistics > --- > > Key: MATH-1426 > URL: https://issues.apache.org/jira/browse/MATH-1426 > Project: Commons Math > Issue Type: Improvement >Affects Versions: 4.0 >Reporter: Karl Richter > Fix For: 4.0 > > Attachments: > 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch, > 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > > > It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`. > The patch is available at https://github.com/apache/commons-math/pull/54 in > form of a PR as well. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Comment Edited] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[
https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114457#comment-16114457
]
Karl Richter edited comment on MATH-1426 at 8/4/17 2:46 PM:
Thanks for your review. You have impressive coding discipline!
> Prefer several small test methods (one for each tested functionality) even if
> some boiler-plate code repetition do occur. Having "testInit1()",
> "testInit2()", ... is fine (but more meaningful names are preferred if
> possible).
Done.
> If using random data, use a fixed seed, unless the behaviour under test has
> intrinsic variability (which is not the case here).
I don't see the downside of using a different "fixed" ("fixed" for the run of
the test, not "fixed" as you mean it) seed for each run of the unit tests, e.g.
based on the test start time millis, and log the seed so that reproduction in
case of test failure is possible since we're working with pseudo-random
generators which can be re-initialized with the same seed to produce the same
pseudo-random results. In case you agree, how do I initialize a Commons RNG
with such a variable seed?
> Make test sets small (as long as they can reasonably check the functionality)
> to avoid long-running "mvn test"; here I don't think that arrays of length
> 1048576 were needed.
Agreed. I chose 1024.
> Comment out debugging output ("System.out.println")
I'm a huge fan of configurable logging framework which on the one hand require
the evaluation of one very cheap statement, but on the other minimize
controversy between devs and avoid adding previously deleted code (since you
can turn logging statements off in your `logback.xml` or whather is used). I
wouldn't speak of "debugging" statements since it's either debugging or
logging. The logging of the generator seed is necessary unless the above is
wrong, I guess you'll shed some light on this issue.
Do you have interest in adding a logging framework (I suggest slf4j-api +
logback-classic). There're about 100 System.out.print statements in the code,
some commented out. I'd provide a patch if you want.
> Apply a uniform coding style (e.g. there must be a space around operators,
> and the tabulation is wrong).
I suggest you move `maven-checkstyle-plugin` from `reporting` to `build` with
{code:java}
checkstyle
check
validate
{code}
That reveals some hundred issues which should either be silenced or fixed
(almost all of them are Javadoc issues which you might deactive for the check
and put on your schedule to fix later). It minimizes communication overhead
before reviewing contributions like in this situation. The issues you're
describing are not revealed by checkstyle and I didn't figure out what you mean
by wrong tabulation - no need to explain if you change the checkstyle, since
then I can rebase the patch.
was (Author: krichter):
Thanks for your review. You have impressive coding discipline!
> Prefer several small test methods (one for each tested functionality) even if
> some boiler-plate code repetition do occur. Having "testInit1()",
> "testInit2()", ... is fine (but more meaningful names are preferred if
> possible).
> For random generation, please use the classes from Commons RNG (cf. examples
> in other test classes).
Done.
> If using random data, use a fixed seed, unless the behaviour under test has
> intrinsic variability (which is not the case here).
I don't see the downside of using a different "fixed" ("fixed" for the run of
the test, not "fixed" as you mean it) seed for each run of the unit tests, e.g.
based on the test start time millis, and log the seed so that reproduction in
case of test failure is possible since we're working with pseudo-random
generators which can be re-initialized with the same seed to produce the same
pseudo-random results. In case you agree, how do I initialize a Commons RNG
with such a variable seed?
> Make test sets small (as long as they can reasonably check the functionality)
> to avoid long-running "mvn test"; here I don't think that arrays of length
> 1048576 were needed.
Agreed. I chose 1024.
> Comment out debugging output ("System.out.println")
I'm a huge fan of configurable logging framework which on the one hand require
the evaluation of one very cheap statement, but on the other minimize
controversy between devs and avoid adding previously deleted code (since you
can turn logging statements off in your `logback.xml` or whather is used). I
wouldn't speak of "debugging" statements since it's either debugging or
logging. The logging of the generator seed is necessary unless the above is
wrong, I guess you'll shed some light on this issue.
Do you have interest in adding a logging framework (I suggest slf4j-api +
logback-classic). There're about 100
[jira] [Commented] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[
https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114457#comment-16114457
]
Karl Richter commented on MATH-1426:
Thanks for your review. You have impressive coding discipline!
> Prefer several small test methods (one for each tested functionality) even if
> some boiler-plate code repetition do occur. Having "testInit1()",
> "testInit2()", ... is fine (but more meaningful names are preferred if
> possible).
> For random generation, please use the classes from Commons RNG (cf. examples
> in other test classes).
Done.
> If using random data, use a fixed seed, unless the behaviour under test has
> intrinsic variability (which is not the case here).
I don't see the downside of using a different "fixed" ("fixed" for the run of
the test, not "fixed" as you mean it) seed for each run of the unit tests, e.g.
based on the test start time millis, and log the seed so that reproduction in
case of test failure is possible since we're working with pseudo-random
generators which can be re-initialized with the same seed to produce the same
pseudo-random results. In case you agree, how do I initialize a Commons RNG
with such a variable seed?
> Make test sets small (as long as they can reasonably check the functionality)
> to avoid long-running "mvn test"; here I don't think that arrays of length
> 1048576 were needed.
Agreed. I chose 1024.
> Comment out debugging output ("System.out.println")
I'm a huge fan of configurable logging framework which on the one hand require
the evaluation of one very cheap statement, but on the other minimize
controversy between devs and avoid adding previously deleted code (since you
can turn logging statements off in your `logback.xml` or whather is used). I
wouldn't speak of "debugging" statements since it's either debugging or
logging. The logging of the generator seed is necessary unless the above is
wrong, I guess you'll shed some light on this issue.
Do you have interest in adding a logging framework (I suggest slf4j-api +
logback-classic). There're about 100 System.out.print statements in the code,
some commented out. I'd provide a patch if you want.
> Apply a uniform coding style (e.g. there must be a space around operators,
> and the tabulation is wrong).
I suggest you move `maven-checkstyle-plugin` from `reporting` to `build` with
{code:java}
checkstyle
check
validate
{code}
That reveals some hundred issues which should either be silenced or fixed
(almost all of them are Javadoc issues which you might deactive for the check
and put on your schedule to fix later). It minimizes communication overhead
before reviewing contributions like in this situation. The issues you're
describing are not revealed by checkstyle and I didn't figure out what you mean
by wrong tabulation - no need to explain if you change the checkstyle, since
then I can rebase the patch.
> Add constructor with Double[] argument to DescriptiveStatistics
> ---
>
> Key: MATH-1426
> URL: https://issues.apache.org/jira/browse/MATH-1426
> Project: Commons Math
> Issue Type: Improvement
>Affects Versions: 4.0
>Reporter: Karl Richter
> Fix For: 4.0
>
> Attachments:
> 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch,
> 0002-added-constructor-with-Double-argument-to-Descriptiv.patch
>
>
> It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`.
> The patch is available at https://github.com/apache/commons-math/pull/54 in
> form of a PR as well.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (NET-641) SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
[
https://issues.apache.org/jira/browse/NET-641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114456#comment-16114456
]
Sebb commented on NET-641:
--
Then either the Javadoc is wrong or the code is wrong
> SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
>
>
> Key: NET-641
> URL: https://issues.apache.org/jira/browse/NET-641
> Project: Commons Net
> Issue Type: Bug
>Affects Versions: 3.6
> Environment: Windows; JDK8; common-net 3.6
>Reporter: pin_ptr
>Priority: Minor
> Fix For: 3.7
>
>
> Code:
> import org.apache.commons.net.util.SubnetUtils;
> public class A {
> public static void main(String[] args) {
> System.out.println(new
> SubnetUtils("192.168.1.0/30").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/31").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/32").getInfo().isInRange("0.0.0.0"));
> }
> }
> Result:
> false
> true
> true
> Expected:
> false
> false
> false
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (NET-641) SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
[
https://issues.apache.org/jira/browse/NET-641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114426#comment-16114426
]
pin_ptr commented on NET-641:
-
import org.apache.commons.net.util.SubnetUtils;
public class B {
public static void main(String[] args) {
SubnetUtils c = new SubnetUtils("192.168.1.0/24");
System.out.println(c.getInfo().isInRange("192.168.1.0"));
System.out.println(c.getInfo().isInRange("192.168.1.255"));
c.setInclusiveHostCount(true);
System.out.println(c.getInfo().isInRange("192.168.1.0"));
System.out.println(c.getInfo().isInRange("192.168.1.255"));
}
}
Javadoc says this code output 4x false, but commons-net 3.6's actual output is
false, false, true and true.
I looked through SVN log and found isInclusiveHostCount was added in r920235
(Mar 2008),
and then low(), high() and isInRange() changed their behavior. (because
isInRange() uses low() and high())
In other words, isInRange never ignored isInclusiveHostCount for 9 years.
I am worried that commons-net changes it's behavior only if
isInclusiveHostCount is true and network address is 0.0.0.0.
> SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
>
>
> Key: NET-641
> URL: https://issues.apache.org/jira/browse/NET-641
> Project: Commons Net
> Issue Type: Bug
>Affects Versions: 3.6
> Environment: Windows; JDK8; common-net 3.6
>Reporter: pin_ptr
>Priority: Minor
> Fix For: 3.7
>
>
> Code:
> import org.apache.commons.net.util.SubnetUtils;
> public class A {
> public static void main(String[] args) {
> System.out.println(new
> SubnetUtils("192.168.1.0/30").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/31").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/32").getInfo().isInRange("0.0.0.0"));
> }
> }
> Result:
> false
> true
> true
> Expected:
> false
> false
> false
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (NET-641) SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
[
https://issues.apache.org/jira/browse/NET-641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114393#comment-16114393
]
Sebb commented on NET-641:
--
isInRange ignores isInclusiveHostCount in order to do what the Javadoc says it
will do.
If there is a use-case for a new method that takes this into account then
please raise an enhancement request.
Ideally with test cases.
> SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
>
>
> Key: NET-641
> URL: https://issues.apache.org/jira/browse/NET-641
> Project: Commons Net
> Issue Type: Bug
>Affects Versions: 3.6
> Environment: Windows; JDK8; common-net 3.6
>Reporter: pin_ptr
>Priority: Minor
> Fix For: 3.7
>
>
> Code:
> import org.apache.commons.net.util.SubnetUtils;
> public class A {
> public static void main(String[] args) {
> System.out.println(new
> SubnetUtils("192.168.1.0/30").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/31").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/32").getInfo().isInRange("0.0.0.0"));
> }
> }
> Result:
> false
> true
> true
> Expected:
> false
> false
> false
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (NET-641) SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
[
https://issues.apache.org/jira/browse/NET-641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114382#comment-16114382
]
pin_ptr commented on NET-641:
-
How about 255.255.255.255 for /0 or x.x.x.0/x.x.x.255 for /24 if
isInclusiveHostCount is true?
> SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
>
>
> Key: NET-641
> URL: https://issues.apache.org/jira/browse/NET-641
> Project: Commons Net
> Issue Type: Bug
>Affects Versions: 3.6
> Environment: Windows; JDK8; common-net 3.6
>Reporter: pin_ptr
>Priority: Minor
> Fix For: 3.7
>
>
> Code:
> import org.apache.commons.net.util.SubnetUtils;
> public class A {
> public static void main(String[] args) {
> System.out.println(new
> SubnetUtils("192.168.1.0/30").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/31").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/32").getInfo().isInRange("0.0.0.0"));
> }
> }
> Result:
> false
> true
> true
> Expected:
> false
> false
> false
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[
https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114362#comment-16114362
]
Gilles commented on MATH-1426:
--
Thanks for your interest in contributing!
We certainly look forward to having your help with the next release of Commons
Math.
A few nit-picks to make the integration of patches smoother:
* Prefer several small test methods (one for each tested functionality) even if
some boiler-plate code repetition do occur. Having "testInit1()",
"testInit2()", ... is fine (but more meaningful names are preferred if
possible).
* For random generation, please use the classes from [Commons
RNG|http://commons.apache.org/proper/commons-rng/] (cf. examples in other test
classes).
* If using random data, use a fixed seed, unless the behaviour under test has
intrinsic variability (which is not the case here).
* Make test sets small (as long as they can reasonably check the functionality)
to avoid long-running "mvn test"; here I don't think that arrays of length
1048576 were needed.
* Comment out debugging output ("System.out.println")
* Apply a uniform coding style (e.g. there must be a space around operators,
and the tabulation is wrong).
> Add constructor with Double[] argument to DescriptiveStatistics
> ---
>
> Key: MATH-1426
> URL: https://issues.apache.org/jira/browse/MATH-1426
> Project: Commons Math
> Issue Type: Improvement
>Affects Versions: 4.0
>Reporter: Karl Richter
> Fix For: 4.0
>
> Attachments:
> 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch,
> 0002-added-constructor-with-Double-argument-to-Descriptiv.patch
>
>
> It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`.
> The patch is available at https://github.com/apache/commons-math/pull/54 in
> form of a PR as well.
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Created] (EXEC-104) Test failure "Exec34Test.testExec34_2:88 Watchdog should have killed the process"
Karl Richter created EXEC-104: - Summary: Test failure "Exec34Test.testExec34_2:88 Watchdog should have killed the process" Key: EXEC-104 URL: https://issues.apache.org/jira/browse/EXEC-104 Project: Commons Exec Issue Type: Bug Reporter: Karl Richter [Travis CI reveals the following test failures](https://travis-ci.org/apache/commons-exec/jobs/237502021): ``` Exec34Test.testExec34_2:88 Watchdog should have killed the process Exec41Test.testExec41WithStreams:96 The process was killed by the watchdog Exec41Test.testExec41WithoutStreams:143 The process was killed by the watchdog ``` If it's a Travis CI issue, the script should be fixed to avoid or suppress these issues. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Resolved] (NET-641) SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
[
https://issues.apache.org/jira/browse/NET-641?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Sebb resolved NET-641.
--
Resolution: Fixed
> SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
>
>
> Key: NET-641
> URL: https://issues.apache.org/jira/browse/NET-641
> Project: Commons Net
> Issue Type: Bug
>Affects Versions: 3.6
> Environment: Windows; JDK8; common-net 3.6
>Reporter: pin_ptr
>Priority: Minor
> Fix For: 3.7
>
>
> Code:
> import org.apache.commons.net.util.SubnetUtils;
> public class A {
> public static void main(String[] args) {
> System.out.println(new
> SubnetUtils("192.168.1.0/30").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/31").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/32").getInfo().isInRange("0.0.0.0"));
> }
> }
> Result:
> false
> true
> true
> Expected:
> false
> false
> false
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Commented] (NET-641) SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
[
https://issues.apache.org/jira/browse/NET-641?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114271#comment-16114271
]
Sebb commented on NET-641:
--
No.
If you look at the Javadoc for isInRange(String), which calls
isInRange(integer), you will see that 0.0.0.0 can never be in the range because
it always excludes the broadcast and network addresses.
Changing that would be a change to the published API.
> SubnetUtils.SubnetInfo.isInRange("0.0.0.0") returns true for CIDR/31, 32
>
>
> Key: NET-641
> URL: https://issues.apache.org/jira/browse/NET-641
> Project: Commons Net
> Issue Type: Bug
>Affects Versions: 3.6
> Environment: Windows; JDK8; common-net 3.6
>Reporter: pin_ptr
>Priority: Minor
> Fix For: 3.7
>
>
> Code:
> import org.apache.commons.net.util.SubnetUtils;
> public class A {
> public static void main(String[] args) {
> System.out.println(new
> SubnetUtils("192.168.1.0/30").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/31").getInfo().isInRange("0.0.0.0"));
> System.out.println(new
> SubnetUtils("192.168.1.0/32").getInfo().isInRange("0.0.0.0"));
> }
> }
> Result:
> false
> true
> true
> Expected:
> false
> false
> false
--
This message was sent by Atlassian JIRA
(v6.4.14#64029)
[jira] [Comment Edited] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[ https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114221#comment-16114221 ] Karl Richter edited comment on MATH-1426 at 8/4/17 10:29 AM: - > A unit test would be welcome. Done. Coverage increased. Travis CI failed because of a crash of the JVM, please restart it. was (Author: krichter): > A unit test would be welcome. Done. Travis CI failed because of a crash of the JVM, please restart it. > Add constructor with Double[] argument to DescriptiveStatistics > --- > > Key: MATH-1426 > URL: https://issues.apache.org/jira/browse/MATH-1426 > Project: Commons Math > Issue Type: Improvement >Affects Versions: 4.0 >Reporter: Karl Richter > Fix For: 4.0 > > Attachments: > 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch, > 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > > > It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`. > The patch is available at https://github.com/apache/commons-math/pull/54 in > form of a PR as well. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Commented] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[ https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=16114221#comment-16114221 ] Karl Richter commented on MATH-1426: > A unit test would be welcome. Done. Travis CI failed because of a crash of the JVM, please restart it. > Add constructor with Double[] argument to DescriptiveStatistics > --- > > Key: MATH-1426 > URL: https://issues.apache.org/jira/browse/MATH-1426 > Project: Commons Math > Issue Type: Improvement >Affects Versions: 4.0 >Reporter: Karl Richter > Fix For: 4.0 > > Attachments: > 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch, > 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > > > It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`. > The patch is available at https://github.com/apache/commons-math/pull/54 in > form of a PR as well. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Updated] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[ https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Karl Richter updated MATH-1426: --- Attachment: (was: 0002-added-constructor-with-Double-argument-to-Descriptiv.patch) > Add constructor with Double[] argument to DescriptiveStatistics > --- > > Key: MATH-1426 > URL: https://issues.apache.org/jira/browse/MATH-1426 > Project: Commons Math > Issue Type: Improvement >Affects Versions: 4.0 >Reporter: Karl Richter > Fix For: 4.0 > > Attachments: > 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch > > > It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`. > The patch is available at https://github.com/apache/commons-math/pull/54 in > form of a PR as well. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
[jira] [Updated] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
[ https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ] Karl Richter updated MATH-1426: --- Attachment: 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > Add constructor with Double[] argument to DescriptiveStatistics > --- > > Key: MATH-1426 > URL: https://issues.apache.org/jira/browse/MATH-1426 > Project: Commons Math > Issue Type: Improvement >Affects Versions: 4.0 >Reporter: Karl Richter > Fix For: 4.0 > > Attachments: > 0001-fixed-javadoc-of-constructors-in-DescriptiveStatisti.patch, > 0002-added-constructor-with-Double-argument-to-Descriptiv.patch > > > It'd be nice to have a `Double[]` constructor in `DescriptiveStatistics`. > The patch is available at https://github.com/apache/commons-math/pull/54 in > form of a PR as well. -- This message was sent by Atlassian JIRA (v6.4.14#64029)
