commons-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Gilles (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (MATH-1426) Add constructor with Double[] argument to DescriptiveStatistics
Date Fri, 04 Aug 2017 15:40:00 GMT

    [ https://issues.apache.org/jira/browse/MATH-1426?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=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)

Mime
View raw message