Possible _ArrayUnique bug?

[Jfish]

One more quick data point, I re-ran my test with $iIntType at the default of 0 - but before I called _ArrayUnique I cast all values in the array to strings and it worked whereas before it failed.

@Jos edit crossed with my post ...

Edited November 20, 2015 by Jfish

[Jos]

There is one other issue with _ArrayUnique: 

This line is not retrieving the correct cell when using _ArrayUnique($sourceArray,1)

Local $vFirstElem = ( ($iDims = 1) ? ($aArray[$iBase]) : ($aArray[$iColumn][$iBase]) )

and think it needs to be changed to:

Local $vFirstElem = ( ($iDims = 1) ? ($aArray[$iBase]) : ($aArray[$iBase][$iColumn]) )

To make the test for the first cell work properly.

Jos

Edited November 20, 2015 by Jos

[Melba23]

Jos,

Good spot with the base/column switch - my fault.

I seem to remember a discussion some time ago about IsInt returning True when used on doubles that held integer values - so I agree that we should use VarGetType to detect when a true integer is used:

; Autocheck of first element
If $iIntType = $ARRAYUNIQUE_AUTO Then
    Local $vFirstElem = ( ($iDims = 1) ? ($aArray[$iBase]) : ($aArray[$iBase][$iColumn]) )
    If StringRegExp(VarGetType($vFirstElem), "Int(32|64)") Then
        Switch VarGetType($vFirstElem)
            Case "Int32"
                $iIntType = $ARRAYUNIQUE_FORCE32
            Case "Int64"
                $iIntType = $ARRAYUNIQUE_FORCE64
        EndSwitch

    Else
        $iIntType = $ARRAYUNIQUE_FORCE32
    EndIf
EndIf

Now doubles fall through and we save a couple of lines on your suggested solution above.

M23

[czardas]

You can get rid of the RegExp.
 

; Autocheck of first element
If $iIntType = $ARRAYUNIQUE_AUTO Then
    Local $vFirstElem = ( ($iDims = 1) ? $aArray[$iBase] : $aArray[$iBase][$iColumn] )
    If StringInStr(VarGetType($vFirstElem), "Int") Then
        Switch VarGetType($vFirstElem)
            Case "Int32"
                $iIntType = $ARRAYUNIQUE_FORCE32
            Case "Int64"
                $iIntType = $ARRAYUNIQUE_FORCE64
        EndSwitch

    Else
        $iIntType = $ARRAYUNIQUE_FORCE32
    EndIf
EndIf

Edit: and perhaps a couple of parentheses.

Edited November 21, 2015 by czardas

[TheDcoder]

I stumbled upon this bug too:

The reproducer script is only 5 lines

[TheDcoder]

I polished @czardas's snippet further:

If $iIntType = $ARRAYUNIQUE_AUTO Then
    Local $vFirstElem = ( ($iDims = 1) ? $aArray[$iBase] : $aArray[$iBase][$iColumn] )
    Switch VarGetType($vFirstElem)
        Case "Int32"
            $iIntType = $ARRAYUNIQUE_FORCE32
        Case "Int64"
            $iIntType = $ARRAYUNIQUE_FORCE64
        Case Else
            $iIntType = $ARRAYUNIQUE_FORCE32
    EndSwitch

Hope its not buggy

[czardas]

@TheDcoder Well spotted!  

If $iIntType = $ARRAYUNIQUE_AUTO Then
    Local $vFirstElem = ( ($iDims = 1) ? $aArray[$iBase] : $aArray[$iBase][$iColumn] )
    Switch VarGetType($vFirstElem)
        Case "Int64"
            $iIntType = $ARRAYUNIQUE_FORCE64
        Case Else
            $iIntType = $ARRAYUNIQUE_FORCE32
    EndSwitch
EndIf

Edit: I'm a bit confused about the logic though. A float double could be greater than 0x7FFFFFFF, or less than 0x80000000. Perhaps I misunderstand the intention.

Edited November 21, 2015 by czardas

[TheDcoder]

@czardas

If $iIntType = $ARRAYUNIQUE_AUTO Then
    Local $vFirstElem = ( ($iDims = 1) ? $aArray[$iBase] : $aArray[$iBase][$iColumn] )
    If VarGetType($vFirstElem) = "Int64" Then
        $iIntType = $ARRAYUNIQUE_FORCE64
    Else
        $iIntType = $ARRAYUNIQUE_FORCE32
    EndIf
EndIf

[czardas]

I think this is not correct. See my previous edit.

[Jos]

 

now doubles fall through and we save a couple of lines on your suggested solution above.

Just made the change in that way to prove my point and leave it to the others to come up with the best way to code this. (As is clearly demonstrate in the subsequent posts  )
I mostly find pleasure in solving the "mystery". 

Jos  

 

[Melba23]

czardas,

I'm a bit confused about the logic though

Then re-read this thread (private forum).

 M23

[czardas]

I looked at the current implementation. I still don't understand why you only want to test the first element, but the logic of the code is flawed regardless. An integer can be of three types. Conversion of doubles to integer datatypes can have two outcomes: Int-32 or Int-64.

Look at __DoubleTo64() in my operator64 (see sig). It's not a perfect solution, but you should see what I mean.

Edited November 21, 2015 by czardas

[Melba23]

czardas,

I still don't understand why you use only want to test the first element

The function cannot cope with every possible case (in a similar fashion to the _FileWritetoLine case we are discussing elsewhere), so the default assumption is made that the elements to be "uniqued" are all of the same datatype. So when the default $ARRAYUNIQUE_AUTO parameter is used, the function looks at the first element to be "uniqued" and determines its datatype. If the element is an Int64 then the function uses the slower algorithm and errors if it finds any Int32 elements - in all other cases it runs the fast algorithm and errors if it finds any Int64 elements. The user can override this by using the other parameters as explained In the Help file. We are forced into having 2 algorithms by the inability of the scripting object to accept Int64 keys and wanting to run the function as fast as possible - the "slow" algorithm being very significantly slower.

If you think this logic is flawed then please do explain why.

M23

[czardas]

I added a comment about doubles to my last post. This is the function I mentioned.

; #INTERNAL_USE_ONLY# ==========================================================================================================
; Name...........: __DoubleTo64
; Description ...: Helper function - converts an integer value double to Int-32 or Int-64.
; Syntax.........: __DoubleTo64($nValue)
; Parameters.....; $nValue - The double to convert
; Return values .: Success returns Int-32 or Int-64 when the interpreter evaluates the converted double equal to the input.
;                  Failure returns the input parameter without any modification and sets the following flags:
;                  |@error = 3 Internal error (x2) ==> see comments in the code.
;                  |@extended = 2 Conversion failed ==> the return value is not an integer.
; Author ........: czardas
; Comments ......; Doubles representing integer values greater than 15 digits cannot be relied on for accuracy.
; ==============================================================================================================================

Func __DoubleTo64($nValue)
    If Not IsNumber($nValue) Then Return SetError(3, 2, $nValue) ; the input is not a number.
    If $nValue > -1.0e+015 And $nValue < 1.0e+015 Then
        Local $iVal64 = Number($nValue, 2) ; convert to Int-64
        If $iVal64 = $nValue Then ; check to see if conversion was a success [expected range +/- 5.62949953421311e+014]
            Return ($iVal64 > 2147483647 Or $iVal64 < -2147483648) ? $iVal64 : Number($iVal64, 1) ; Int-64 or Int-32
        ElseIf $iVal64 -1 = $nValue Then ; attempt to adjust for inaccuracies [subject to possible change]
            Return $iVal64 -1
        ElseIf $iVal64 +1 = $nValue Then ; as above
            Return $iVal64 +1
        EndIf
    EndIf

    Return SetError(3, 2, $nValue) ; conversion failed
EndFunc ;==> __DoubleTo64

If you automatically assume Int-32 for an integer that is not Int-64, I guess it will max out on a large percentage of doubles.

Edited November 21, 2015 by czardas

[Melba23]

czardas,

There has to be a balance between user convenience and the function's ability to cover every possible edge case. As the Help file explains, if the user knows (or even suspects) that there are large double values involved then they can set the $iIntType parameter to use the slower algorithm and thus not force any integer-value doubles into Int32. Getting the function to check every element in an attempt to determine the correct fashion to deal with the contents of an array will make it even slower than it already is.

M23

[czardas]

I haven't used the scripting object method, so I will take your word for it. My suggested corrections to that line would be:

If $iIntType = $ARRAYUNIQUE_AUTO Then
    Local $vFirstElem = ( ($iDims = 1) ? $aArray[$iBase] : $aArray[$iBase][$iColumn] )
    If VarGetType($vFirstElem) = "Int64" Or $vFirstElem > 0x7FFFFFFF Or $vFirstElem < 0x80000000 Then
        $iIntType = $ARRAYUNIQUE_FORCE64
    Else
        $iIntType = $ARRAYUNIQUE_FORCE32
    EndIf
EndIf

This shouldn't add any significant delay, other than when it is actually needed (but it's obviously not foolproof).

Edited November 21, 2015 by czardas

[BrewManNH]

I'd make it do as little as possible to produce the desired results and leave the edge cases to the person writing their script to overcome the limitations. There seems to be a function inflation in regards to every UDF these days that just shouldn't be done. They're used as a library function, they shouldn't be customized to every user's whim. Get them, the included UDFs, doing the basics, and leave the inflation to the user's to devise their own functions.

[czardas]

Large integer doubles are not exactly what I would call edge cases. As soon as you use the division operator, you get a double. That being said, I agree that a compromise has to be made somewhere. Recursively looping through multidimensional arrays contained within the elements of other arrays to test all elements within a main (non-unique) array would be taking things way too far IMO.

Edited November 21, 2015 by czardas

[BrewManNH]

My comment wasn't referencing yours, its in reference to the whole thread heading towards the function being redone to accommodate the oddest of cases instead of leaving it alone and letting it do what it's supposed to do with the standard functionality. As I said, let the edge cases be handled by the person writing the script if they want it, library functions shouldn't be handling EVERY possible possibility.

[czardas]

Agreed! I think strings and numbers are the most used datatype and these datatypes probably should receive the most attention. I mean generally for this kind of function.

Edited November 21, 2015 by czardas