Resolves issue #179 (https://github.com/alimanfoo/petl/issues/179): sear... #274
Conversation
…lement searchcomplement() function has been added. For code reusability and to implicitly ensure that the function is the complement of search, the implementation is wrapper of search(), and search() has been extended to allow for complement behavior. search() (and related views and iterator functions) now includes a 'complement_flag' boolean (default=False). This yields the existing functionality (confirmed with existing unittests). When complement_flag=True, the iterator function now yields rows that fail the test() lambda function Docstring modified to describe functionality, mirroring the docstring for search() unittest added: petl.test.transform.text_regex.test_searchcomplement()
|
Hi Roger, implementation looks good, I'm just thinking about API consistency. There is a parallel with the petl.select() function, which accepts a "complement" keyword argument. For consistency I suggest renaming the new search "complement_flag" keyword argument to just "complement". I'm also now wondering in hindsight whether it's worth having a searchcomplement() function at all, as it doesn't add much convenience over calling search(..., complement=True). I don't feel strongly about it, but I could easily live without searchcomplement(), and moving the nice examples from the docstring you created into the existing docstring for search() to describe the behaviour of the "complement" keyword argument. |
|
I considered the search(..., complement=True) option - it's definitely easy enough to expose either/or/both. From a consistency perspective, I can see it both ways. I couldn't find an analogous situation in the existing codebase, so we're venturing into new territory a bit :-). There's already complement(), hashcomplement(), and recordcomplement(). While those are using the term in the more specific set theory concept of the word, there's some consistency in having searchcomplement() as a name. Specifying it as an optional argument to search() might seem out of place - return X OR return ~X. I modified the docstring to link between the two :func:, which will hopefully aid the documentation |
| @@ -293,11 +293,16 @@ def search(table, *args, **kwargs): | |||
| | 'mango' | 42 | 'I like them' | | |||
| +----------+-------+--------------------------+ | |||
|
|
|||
| The complement of search() (i.e., the rose not found via search()) | |||
There was a problem hiding this comment.
That which by any other name would smell just as sweet :-)
There was a problem hiding this comment.
Hah, don't know what I was thinking there
|
Thanks Roger, minor slip in the docstring, otherwise looks good to go. |
Fixed docstring - although "data rose" sounds like something I should buy my wife, we're actually referring to "data rows" here.
|
The last you'll see of the "data rose" : @}-,-`- |
|
I know a bank where the wild thyme blows, |
Travis tests passed
search complement
searchcomplement() function has been added. For code reusability and to implicitly ensure that the function is the complement of search, the implementation is wrapper of search(), and search() has been extended to allow for complement behavior.
search() (and related views and iterator functions) now includes a 'complement_flag' boolean (default=False). This yields the existing functionality (confirmed with existing unittests).
When complement_flag=True, the iterator function now yields rows that fail the test() lambda function
Docstring modified to describe functionality, mirroring the docstring for search()
unittest added: petl.test.transform.text_regex.test_searchcomplement()