ibis
ibis copied to clipboard
test: fix ArrayValue.remove() handling of NULLs
ibis.array([1, None, 2]).remove(None)
doesn't behave as expected. Still a WIP, figuring out what should be desired.
Sorry, this is very much a WIP, should have marked it as such. Added a very basic description to the OP.
I think the postgres choice of using IS NOT DISTINCT FROM
semantics in remove is inconsistent with NULL
input behavior in nearly every other function and API.
If an argument is NULL
, that means it's unknown and for consistency the only sane thing to do would be to return NULL
in such cases.
Now, it's important to have convenience APIs, for example for removing all NULLs in an array.
I propose that we make the semantics of Ibis's Array.remove
match those of Trino (which returns NULL
when NULL
is the element to search for), and add a new API compress
(?) that strips all nulls from an array.
@cpcloud just to be clear, currently ibis.array([1, None, 2]).remove(2)
results in [1]
. In this PR I just want to fix it so this results in [1, None]
. Were you thinking I was trying to make Array.remove(None)
strip NULLs? I am happy with not supporting that, and expecting users to do Array.filter(_.notnull())
Ah, yep, I thought that's what you had in mind. Glad to see we're in agreement here!
ACTION NEEDED
Ibis follows the Conventional Commits specification for release automation.
The PR title and description are used as the merge commit message.
Please update your PR title and description to match the specification.
Yup that's my bad for a vague PR title and no description (I edited it to be more clear after I saw your confusion, you're not being gaslit here, it WAS confusing)
Thanks @cpcloud for finishing this up!