The roundup "filter" method (see hyperdb.py) has a "search_matches"
parameter that can be used to constrain the search.  In particular,
only elements which otherwise match the search *and* are in
search_matches will be considered.

Conceptually, 'search_matches' is a set. However, as some backend code
calls 'has_key()' on it, it needs to be a dictionary.

The attached patch simplifies this, so any sequence type may be used.

OK to check in ?

Absolutely go for it.

     Richard

Sent from my expensive toy

On 17/02/2009, at 9:27 AM, Stefan Seefeld <issues@roundup-tracker.org>  
wrote:

>
> New submission from Stefan Seefeld <stefan@codesourcery.com>:
>
> The roundup "filter" method (see hyperdb.py) has a "search_matches"
> parameter that can be used to constrain the search.  In particular,
> only elements which otherwise match the search *and* are in
> search_matches will be considered.
>
> Conceptually, 'search_matches' is a set. However, as some backend code
> calls 'has_key()' on it, it needs to be a dictionary.
>
> The attached patch simplifies this, so any sequence type may be used.
>
> OK to check in ?
>
> ----------
> assignee: stefan
> files: search_matches.patch
> keywords: patch
> messages: 3550
> nosy: richard, stefan
> severity: normal
> status: new
> title: Simplify 'search_matches' argument to filter()
> type: rfe
> versions: devel
>
> ________________________________________________
> Roundup tracker <issues@roundup-tracker.org>
> <http://issues.roundup-tracker.org/issue2550505>
> ________________________________________________

Fixed with rev:4136

The patch is incorrect. It leaves the line

  args = args + v

even though the variable v has no value anymore.

I fail to see where precisely search_matches is changed to be a
sequence. My testing shows it remains a dict. So the correct line should be

  args = args + search_matches.keys()

However, this is actually what it was before, so I'm puzzled...

Martin v. Löwis @psf.upfronthosting.co.za wrote:

> The patch is incorrect. It leaves the line
> 
>   args = args + v
> 
> even though the variable v has no value anymore.

I'd propose this fix:

-            args = args + v
+            args = args + [x for x in search_matches]

as the intention of the original patch was to use 'in' so any container 
type would be possible. Richard, how does this look ?

Thanks,
		Stefan

Martin v. Löwis @psf.upfronthosting.co.za wrote:

> The patch is incorrect. It leaves the line
> 
>   args = args + v

Where ?

> 
> even though the variable v has no value anymore.
> 
> I fail to see where precisely search_matches is changed to be a
> sequence. My testing shows it remains a dict.

The 'search_matches' object comes from (tracker instance) template code. 
All I meant to change is its handling, so it didn't have to be a dict, 
but could be any container type. (Using 'in' instead of 'has_key', to be 
more polymorphic.)

Thanks,
		Stefan

> I'd propose this fix:
> > 
> > -            args = args + v
> > +            args = args + [x for x in search_matches]
> > 
> > as the intention of the original patch was to use 'in' so any container 
> > type would be possible. Richard, how does this look ?

If it is ok, I think there are a few less complicated spellings, such
as

   args = args + list(search_matches)
   args.extend(search_matches)

Martin v. Löwis @psf.upfronthosting.co.za wrote:

>> I'd propose this fix:
>>> -            args = args + v
>>> +            args = args + [x for x in search_matches]

I'v checked in the above yesterday, while the tracker machine was down.

>>> as the intention of the original patch was to use 'in' so any container 
>>> type would be possible. Richard, how does this look ?
> 
> If it is ok, I think there are a few less complicated spellings, such
> as
> 
>    args = args + list(search_matches)
>    args.extend(search_matches)

Ah, I didn't know the above were valid ways to get the same effect. In 
any case, I believe it's fixed in trunk now. Please confirm, so we can 
close this issue.

Thanks,
		Stefan

Hi Stefan, you wrote that you checked this patch in, esp. with this piece:

>> I'd propose this fix:
>>> -            args = args + v
>>> +            args = args + [x for x in search_matches]

I can't find this anywhere on trunk, so where did you check this in, please?

(Running 'svn up' gives me r4209 and a message that I'm up to date.)

sorry... please disregard my last message

Toni Mueller wrote:
> Toni Mueller <dev@tonimueller.org> added the comment:
> 
> Hi Stefan, you wrote that you checked this patch in, esp. with this piece:
> 
>>> I'd propose this fix:
>>>> -            args = args + v
>>>> +            args = args + [x for x in search_matches]
> 
> I can't find this anywhere on trunk, so where did you check this in, please?

On trunk (rev:4205)

http://svn.roundup-tracker.org/viewvc/roundup/roundup/trunk/roundup/backends/rdbms_common.py?r1=4187&r2=4205&pathrev=4205