Mailing-List: contact dev-help@commons.apache.org; run by ezmlm
Precedence: bulk
Reply-To: "Commons Developers List" <dev@commons.apache.org>
Received-SPF: pass (athena.apache.org: domain of phil.steitz@gmail.com
 designates 209.85.210.43 as permitted sender)
Message-ID: <50C75744.4000409@gmail.com>
Date: Tue, 11 Dec 2012 07:54:44 -0800
From: Phil Steitz <phil.steitz@gmail.com>
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.7;
 rv:17.0) Gecko/17.0 Thunderbird/17.0
MIME-Version: 1.0
To: Commons Developers List <dev@commons.apache.org>
Subject: Re: svn commit: r1420000 - in /commons/proper/math/trunk/src:
 changes/changes.xml
 main/java/org/apache/commons/math3/random/EmpiricalDistribution.java
 test/java/org/apache/commons/math3/random/EmpiricalDistributionTest.java
References: <20121211053608.A753223889E0@eris.apache.org>
 <20121211112633.GK20488@dusk.harfang.homelinux.org>
In-Reply-To: <20121211112633.GK20488@dusk.harfang.homelinux.org>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

On 12/11/12 3:26 AM, Gilles Sadowski wrote:
> Hi.
>
> On Tue, Dec 11, 2012 at 05:36:07AM -0000, psteitz@apache.org wrote:
>> Author: psteitz
>> Date: Tue Dec 11 05:36:06 2012
>> New Revision: 1420000
>>
>> URL: http://svn.apache.org/viewvc?rev=1420000&view=rev
>> Log:
>> Added RealDistribution methods to EmpiricalDistribution.  JIRA: MATH-672.
>>
>> Modified:
>>     commons/proper/math/trunk/src/changes/changes.xml
>>     commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/EmpiricalDistribution.java
>>     commons/proper/math/trunk/src/test/java/org/apache/commons/math3/random/EmpiricalDistributionTest.java
>>
>> Modified: commons/proper/math/trunk/src/changes/changes.xml
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/changes/changes.xml?rev=1420000&r1=1419999&r2=1420000&view=diff
>> ==============================================================================
>> --- commons/proper/math/trunk/src/changes/changes.xml (original)
>> +++ commons/proper/math/trunk/src/changes/changes.xml Tue Dec 11 05:36:06 2012
>> @@ -77,6 +77,10 @@ This is a minor release: It combines bug
>>    2. A few methods in the FastMath class are in fact slower that their
>>    counterpart in either Math or StrictMath (cf. MATH-740 and MATH-901).
>>  ">
>> +      <action dev="psteitz" type="update" issue="MATH-672">
>> +        Added methods to EmpiricalDistribution to implement the RealDistribution
>> +        interface.
>> +      </action>
>>        <action dev="tn" type="add" issue="MATH-897" due-to="Reid Hochstedler">
>>          DBSCAN clustering algorithm (in package "o.a.c.m.stat.clustering").
>>        </action>
>>
>> Modified: commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/EmpiricalDistribution.java
>> URL: http://svn.apache.org/viewvc/commons/proper/math/trunk/src/main/java/org/apache/commons/math3/random/EmpiricalDistribution.java?rev=1420000&r1=1419999&r2=1420000&view=diff
>> ==============================================================================
>> [...]
>> +
>> +    /**
>> +     * Mass of bin i under the within-bin kernel of the bin.
>> +     *
>> +     * @param i index of the bin
>> +     * @return the difference in the within-bin kernel cdf between the
>> +     * upper and lower endpoints of bin i
>> +     */
>> +    @SuppressWarnings("deprecation")
>> +    private double kB(int i) {
>> +        final double[] binBounds = getUpperBounds();
>> +        final RealDistribution kernel = getKernel(binStats.get(i));
>> +        return i == 0 ? kernel.cumulativeProbability(min, binBounds[0]) :
>> +            kernel.cumulativeProbability(binBounds[i - 1], binBounds[i]);
>> +    }
> Is there any advantage to suppressing the deprecation warnings?
> There are many other instances in the code. It could be construed that it
> points to the current state of affairs: deprecated code is only used where
> we cannot do otherwise because of backward-compatibility constraints.

I am fine getting rid of the suppressions.  You are correct that the
suppression is there only because there is no way to avoid the warning.
>
>
> Unrelated but in that same file:
>  * The Javadoc mentions "RandomDataImpl" instead of "RandomDataGenerator".

Thanks.  Need to fix that.
>  * The Javadoc for constructor (line 191)
>      public EmpiricalDistribution(int binCount,
>                                   RandomDataGenerator randomData)
>   
>    mentions that "randomData [...] may be null, resulting in default JDK
>    generator". But that instead results in NPE.

Hmm.  I now see that introducing the super(generator) call to avoid
deprecation warning broke the old contract.  I will fix that.  Good
catch.
>  * The constructor (line 164)
>      public EmpiricalDistribution(int binCount,
>                                   RandomGenerator generator)
>    should call the one at line 191 instead of duplicating code.

Yes.  Will fix.
>
>
> Regards,
> Gilles
>
>> [...]
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
> For additional commands, e-mail: dev-help@commons.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@commons.apache.org
For additional commands, e-mail: dev-help@commons.apache.org