> Interestingly, I had originally implemented the is_in method on references
> using the Or expression but I later found it preferable to use the In
> expression. However, in order to support a wider range of backends, I'm fine
> with reverting to the Or expression.
>
> Thanks for suggesting to test the corner case when passing an empty list. I
> made sure to write the corresponding test which then failed when just uring
> the Or expression. So, when the given argument to the is_in method is an empty
> list, it simply returns False which seems to work fine.
>
> The branch has been updated with these changes.
With your new changes, passing an Expr object to is_in() will now fail. With the previous version of your patch it was possible to do something like:
> Interestingly, I had originally implemented the is_in method on references
> using the Or expression but I later found it preferable to use the In
> expression. However, in order to support a wider range of backends, I'm fine
> with reverting to the Or expression.
>
> Thanks for suggesting to test the corner case when passing an empty list. I
> made sure to write the corresponding test which then failed when just uring
> the Or expression. So, when the given argument to the is_in method is an empty
> list, it simply returns False which seems to work fine.
>
> The branch has been updated with these changes.
With your new changes, passing an Expr object to is_in() will now fail. With the previous version of your patch it was possible to do something like:
Employee. company. is_in(Select( Company. id, Company. name.like( "%corp% ")))
The cast to list() in your new version would fail here.