Hi.
2019-08-19 17:10 UTC+02:00, Alex Herbert <alex.d.herb...@gmail.com>:
>
>> On 19 Aug 2019, at 15:45, akash srivastava <akashsp...@gmail.com> wrote:
>>
>> I was advised to check with the dev mailing list if anyone has a problem
>> with the suggested fix.
>> It seems no one does have a problem with the fixes suggested so I'll go
>> forward and create a PR that returns an empty array for an array of all
>> NaNs.
>
> I suggested doing the same as what happens when you pass in an empty array,
> effectively changing the array of NaNs to an array of nothing.
>
> Reading the code it appears that this case would throw an
> ArrayIndexOutOfBoundsException. A simple test case shows that this is true,
> e.g. this passes:
>
> @Test(expected=ArrayIndexOutOfBoundsException.class)
> public void testEmpty() {
> new NaturalRanking().rank(new double[0]);
> }
>
> So the fix for rank() on an array of NaNs has no equivalent with a normal
> array as an empty normal array is an edge case currently not handled.
>
> IMO returning an empty array is not going to be useful. If someone is using
> this class to rank data then immediately after ranking I would assume they
> will use the rank to do something, e.g. pick the top one. An empty array
> will fail for most use cases I can think of since they will require
> dereferencing the rank array. Only if the array is used without
> dereferencing it, e.g. print to file, or if the size of the rank array is
> checked before using the contents will an empty array be OK.
>
> So the question becomes which is better:
>
> - Returning an empty array and requiring users to check its length or else
> receive ArrayIndexOutOfBoundsException when using the array contents
> - Throwing an exception so that the exact cause of their bug can be recorded
> in the stack trace
>
> I would suggest that an array of NaNs or an empty array should throw the
> same and it should be some sort of IllegalArgumentException. Currently this
> class only throws NotANumberException for errors. Perhaps for these two
> cases an math4.exception.InsufficientDataException should be thrown. This
> basically states that you cannot rank nothing and any result that is
> returned will contain a ranking.
>
> Opinions?
It also seems to me that throwing an exception is more in line with
what has usually been done in CM (i.e. "better safe than sorry").
Best regards,
Gilles
>>
>> On Thu, Aug 15, 2019 at 11:25 AM akash srivastava <akashsp...@gmail.com>
>> wrote:
>>
>>> Here is the link for the related bug:
>>> https://issues.apache.org/jira/browse/MATH-1495
>>>
>>> I have suggested two possible fixes when NaturalRanking#rank() is called
>>> on an array of all NaNs:
>>> 1) throwing an IllegalArgumentException
>>> 2) Returning an empty array
>>>
>>> I want to create a pull request regarding it, which fix do you suggest I
>>> should go for?
>>>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@commons.apache.org
For additional commands, e-mail: dev-h...@commons.apache.org
