[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [pygame] BUG: The behavior of the Group.has method vs. its documentation



PS. Actually, I just realized that Group.has only checks the membership of the first sprite when I call grp1.has(spr1, spr2). It's not checking the membership of any of the arguments after the first one. This makes me even more certain that it would be preferable to change the code.


----- Original Message ----
From: Jason M. Marshall <jmm0@xxxxxxxxx>
To: pygame-users@xxxxxxxx
Sent: Mon, February 1, 2010 6:28:59 AM
Subject: Re: [pygame] BUG: The behavior of the Group.has method vs. its  documentation

Unfortunately, correcting the documentation will make it complicated. If we correct the documentation, then we'll have to explain why Group.has checks for membership of ANY sprites that are passed as arbitrary arguments but checks for membership of ALL sprites that are passed as objects within an iterable.

>>> grp1.has(spr1, spr2)
True
>>> grp1.has([spr1, spr2])
False

Since the actual implementation is nuanced, would it still be best to update the documentation rather than change the code? (We could consider the nuance a feature!)

unittests are a good idea. I'll add those.

Jason

----- Original Message ----
From: René Dudfield <renesd@xxxxxxxxx>
To: pygame-users@xxxxxxxx
Sent: Mon, February 1, 2010 5:42:38 AM
Subject: Re: [pygame] BUG: The behavior of the Group.has method vs. its  documentation

On Mon, Feb 1, 2010 at 7:15 PM, Jason M. Marshall <jmm0@xxxxxxxxx> wrote:
> In the pygame.sprite module, I think that the Group.has method does not behave according to its documentation. The online documentation states that Group.has will return True if the Group contains ALL of the given sprites. However, in a certain case, Group.has will return True if the Group contains ANY of the given sprites. In the following interactive code example, grp1.has(spr1, spr2) should return False, but it returns True:
>
>>>> import pygame
>>>> spr1 = pygame.sprite.Sprite()
>>>> spr2 = pygame.sprite.Sprite()
>>>> grp1 = pygame.sprite.Group(spr1)
>>>> grp1.has(spr1)
> True
>>>> grp1.has(spr2)
> False
>>>> grp1.has(spr2, spr1)
> False
>>>> grp1.has(spr1, spr2)
> True
>>>>
>
> I'm already in the process of tidying up the pygame.sprite module so that it'll make fewer function calls, make fewer hash table look-ups and conform to PEP 8 better. So far, I haven't made any changes that could break any existing code, but if I change the AbstractGroup.has code to match the documentation, then someone's game could break if it depends on the incorrect behavior of Group.has.
>
> Would it be OK with all of you if I change Group.has to match the documentation?
>
> Thanks,
> Jason
>


Best to fix the docs, and add another method.

Also, there are not full unittests for the sprite module, so it would
be good to get some unittests completed first... so that it would be
sure to catch any errors with your refactoring.


cu,