larry-discuss team mailing list archive

[josef.pktd@xxxxxxxxx]

On Fri, Feb 5, 2010 at 12:01 PM, Keith Goodman <kwgoodman@xxxxxxxxx> wrote:
> On Fri, Feb 5, 2010 at 8:52 AM, Keith Goodman <kwgoodman@xxxxxxxxx> wrote:
>> On Fri, Feb 5, 2010 at 7:28 AM, Keith Goodman <kwgoodman@xxxxxxxxx> wrote:
>>> On Fri, Feb 5, 2010 at 7:24 AM,  <josef.pktd@xxxxxxxxx> wrote:
>>>> The current signature
>>>>
>>>> assert_larry_equal(actual, desired, msg='', dtype=True, original=None,
>>>>                       noreference=True, nocopy=False)
>>>>
>>>> has 3 keywords, original, noreference, nocopy for the option to check
>>>> whether a larry is a view or a copy.
>>>>
>>>> This makes 8 binary options, only 3 are relevant cases, the other 5
>>>> combinations raise errors or are redundant. This was quite confusing
>>>> when I used the tests, since I often forgot to use, reset a keyword,
>>>> e.g. nocopy=True also requires setting noreference = False
>>>>
>>>> I think a more compact signature would be
>>>> assert_larry_equal(actual, desired, msg='', dtype=True, original=None,
>>>> noref=True)
>>>>
>>>> if not original is None:
>>>>    if noref:
>>>>       check noreference
>>>>    else:
>>>>       check nocopy
>>>>
>>>> otherwise don't do any reference/copy check
>>>>
>>>> the presence of original would indicate that we want the view check,
>>>> `noref` would tell which one.
>>>
>>> This is how larry gets improved---with cleaning like that.
>>>
>>> I'll make the change in the trunk.
>>
>> I changed the function signature as you suggested. I also changed
>> noref to iscopy. The change in the kw name will mess up your unit
>> tests :(

I will change them

>>
>> And I change the name of assert_noreference to assert_iscopy and
>> assert_nocopy to assert_isview.
>
> Here's a potential problem if the label contains objects:
>
>>> import datetime
>>> x = larry([1, 2] , [[9, datetime.date(2009,1,1)]])
>>> y = larry([1, 2] , [[7, x.label[0][1]]])
>>> x.label[0][1] is y.label[0][1]
>   True
>>> la.util.testing.assert_iscopy(x, y)
>>>
>
> assert_iscopy and assert_isview only compare the entire axis at once:
>
>    for i in xrange(larry1.ndim):
>        if larry1.label[i] is larry2.label[i]:
>            msg.append('The labels along axis %d share a reference.' % i)
>
> So I should change that to loop over each label element, right?

Yes. Because this is for the tests, we want to catch all cases, even
if they don't show up in common usage.

Do we have the same problem with nested labels, e.g. tuples after
flatten?  Maybe then a recursive helper function is useful.

Josef