WIP Add lifted version of split by davidanthoff · Pull Request #34 · queryverse/DataValues.jl · GitHub
Skip to content

WIP Add lifted version of split#34

Open
davidanthoff wants to merge 1 commit into
masterfrom
split
Open

WIP Add lifted version of split#34
davidanthoff wants to merge 1 commit into
masterfrom
split

Conversation

@davidanthoff

Copy link
Copy Markdown
Member

This is probably a horrible idea and most likely I won't merge it. But hey, lets think about it a while. It is the only way that I can think of that would make the last query in queryverse/Query.jl#134 work out of the gate without a need to think about missing values... Maybe that goal is mistaken in the first place, though.

Essentially what this would do is treat a missing string passed to split as equivalent to an empty string...

@codecov-io

codecov-io commented Nov 24, 2017

Copy link
Copy Markdown

@tcovert

tcovert commented Nov 24, 2017

Copy link
Copy Markdown

I sorta think that if you go down this path, you ought to add lifted versions of many other string functions in the standard library: length, sizeof, invalid, all the regex stuff etc. I would totally be a fan of this.

@davidanthoff

Copy link
Copy Markdown
Member Author

Oh, I definitely want to add all those lifted versions!

The question is, what should they return when they encounter a NA. There is the philosophy that they should always propagate NA. That is nice and consistent and predictable. But I'm not sure really helpful for something like split, i.e. if the return value could either be an array or NA. But maybe that is what it should do... I'm just not sure.

@tcovert

tcovert commented Nov 24, 2017

Copy link
Copy Markdown

Ah good point. Is there any common theme across base string functions that return scalars vs arrays? In the array case (i.e. ‘split’) where the correct return value for a non-null input is, say, an empty array, returning the same thing for a null input seems reasonable. For the scalar case maybe null propagation makes more sense?

@tcovert

tcovert commented Nov 24, 2017

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants