← Back to team overview

larry-discuss team mailing list archive

Re: stream lining signature of assert_larry_equal

 

On Fri, Feb 5, 2010 at 9:19 AM,  <josef.pktd@xxxxxxxxx> wrote:
> 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.

Now I know why I didn't check each label element:

>>> 9 is 9
True

So I need to check if the object is immutable. Do you know how to do
that? A tuple is immutable, but not the elements inside. Seems
complicated.



Follow ups

References