make `coalesce` handle only `missing`; add `something` #27258

JeffBezanson · 2018-05-25T03:57:51Z

This is a possible approach to fixing #26927. Overall I like it. The name something is ok, but didn't feel perfect. Basically all uses of coalesce in Base were for providing default values for find or match functions, and something(findnext(...), 0) doesn't read quite right. I also notice that it's very similar to notnothing, and the functions could perhaps be combined (one way or the other).

nalimilan

Thanks! Looks reasonable to me.

nalimilan · 2018-05-26T09:25:13Z

base/missing.jl

+julia> coalesce(1, missing)
+1
+
+julia> coalesce(nothing, 1)  # returns `nothing`


Probably better remove this example, there's no reason to think that nothing is treated differently from any other value.

I think the point of the example is to demonstrate that.

Yes, I understand that, but my point is that nobody would have imagined coalesce would consider nothing as a missing value, except for the fact that it was the previous behavior. And since most people won't know that history...

Anyway, not a big deal.

nalimilan · 2018-05-26T09:28:55Z

base/some.jl

-"""
-notnothing(x::Any) = x
-notnothing(::Nothing) = throw(ArgumentError("nothing passed to notnothing"))
+something() = throw(ArgumentError("No value arguments present"))


Same remark as for the zero-argument coalesce below.

nalimilan · 2018-05-26T09:32:14Z

base/some.jl


 """
-    coalesce(x, y...)
+    notnothing(x)


As you noted, I think we can just remove notnothing and use something instead. I don't think we use it in situations where accidentally unwrapping Some is possible. On the contrary, I only used notnothing in places where Nullable previously required explicit unwrapping, which ensured null values were caught early. Anyway it's internal.

nalimilan · 2018-05-26T09:32:49Z

doc/src/base/base.md

 Base.coalesce
 Base.ismissing
 Base.skipmissing
+Base.something


Should be listed with nothing and Some instead, it's totally unrelated to missing.

nalimilan · 2018-05-26T09:36:22Z

base/missing.jl

+"""
+function coalesce end
+
+coalesce() = missing


I'd find it more natural to define coalesce(x::Missing) = throw(...). There's no reason to define a function with zero arguments AFAICT. Same remark for something.

This isn't ancient Rome; we understand zero now 😁

I 100% agree with Jeff here. Zero things is missing the same way the sum of no things is zero and the product of no things is one. If we knew the type in those cases, we would absolutely define the empty sums and products to do that; in this case we do know the type and can return missing.

OK, why not, though I'm not sure in what cases it could be useful. ~~OTOH I don't think it makes sense for something (since an error is thrown), but let's discuss it in the other thread above.~~ EDIT: Actually now I see it's somewhat similar, even though it's not that useful since an error is thrown anyway.

nalimilan · 2018-05-27T09:47:37Z

I also realized it would be nice to have a deprecation for coalesce with nothing, given that the deprecations for search & find methods have recommended using coalesce. Without it, people who have ported their packages early will be left with broken code without any indication about what to do.

nalimilan · 2018-05-27T20:27:31Z

I've rebased and added a commit moving the docs. I'm not sure how to correctly add a deprecation, given that the behavior has changed in an incompatible way. We could just print a warning pointing to something when coalesce is passed nothing or Some.

ViralBShah · 2018-05-27T22:42:11Z

Why did freebsd not happen here?

ViralBShah · 2018-05-28T12:41:40Z

This PR was all green but the conflicts are out of merging the statistics-stdlib PR. I think we should try and merge it today, if the PR is otherwise ready.

…hing`/`Some` fixes #26927

ViralBShah · 2018-05-28T12:55:10Z

Rebased on master. Might as well let CI have another go at it.

@iblis17 FreeBSD doesn't seem to be picking this up.

iblislin · 2018-05-28T16:36:23Z

@ViralBShah I think it built already. just queuing before.

ViralBShah · 2018-05-28T19:31:56Z

Thanks @iblis17. So all green! Is something the name that everyone wants and settled?

JeffBezanson · 2018-05-28T19:43:46Z

I think I might prefer notnothing --- it's very clear, since this returns the first value that's not nothing.

JeffBezanson · 2018-05-28T19:58:34Z

I believe F# calls this defaultArg. I also find that appealing, since this can be described as supplying a default value when a certain value is absent. Maybe defaultval.

JeffBezanson · 2018-05-28T20:01:45Z

Another data point: fromMaybe in Haskell.

nalimilan · 2018-05-28T20:03:52Z

FWIW, Scala uses orElse. EDIT: and Rust uses unwrap (single argument) and unwrap_or (two arguments).

something and notnothing sound more explicit than other choices to me. Maybe let's have a quick poll?

JeffBezanson · 2018-05-28T20:05:30Z

Scala also has getOrElse. Kind of surprising how little agreement there is on what this function is called.

nalimilan · 2018-05-28T20:08:10Z

Yeah. The terminology "null-coalescing operator" is more standard, but even though ?? is the most common there's also a lot of variation.

JeffBezanson · 2018-05-30T16:07:49Z

How about we go with orelse? It's short, and findfirst(...) orelse 0 reads well.

quinnj · 2018-05-30T16:18:14Z

I like orelse

nalimilan · 2018-05-30T16:49:08Z

I'm not a big fan of orelse. It doesn't read very well in the function form orelse(x, y). Also if we want an infix operator, why not use ???

ViralBShah · 2018-05-30T16:59:51Z

I suggest merging this as is for now, in order to not block the alpha, and changing the name to something else should we want to before the release candidate. something seems better than coalesce to me, and about as good as many of the names suggested here.

nalimilan · 2018-05-30T18:38:15Z

Personally, I'm fine with something, but I think we'd better make a hard decision on the API before tagging the alpha. Else people are going to port their packages to the new API (which is used in many places due to to the find & search deprecations), only to find out they need to change it again for the beta.

ViralBShah · 2018-05-30T18:44:01Z

I am going to merge this now. We still have the rest of the day or maybe even tomorrow to pick a different name.

yurivish · 2018-05-30T18:45:51Z

I like ??.

StefanKarpinski · 2018-05-30T19:02:55Z

After some initial skepticism, I've come to like ?? for this operation but that seems to push very hard in the direction of using ? for nothingness rather than missingness, which would likely include T? as a syntax for Union{T, Nothing} as opposed to Union{T, Missing}.

KristofferC · 2018-05-30T19:04:40Z

which would likely include T? as a syntax for Union{T, Nothing} as opposed to Union{T, Missing}.

Totally think that is a better choice.

StefanKarpinski · 2018-05-30T19:09:07Z

That would dovetail with a whole other suite of ? syntaxes that I think are pretty useful:

T? for Union{T, Nothing}
x ?? default for something(x, default)
x ?= default for x = something(x, default)
d[k] ?? default for get(()->default, d, k)
d[k] ?= default for get!(()->default, d, k)

The collection of them is fairly compelling since they're all nothingesque rather than missingesque.

StefanKarpinski · 2018-05-30T19:14:10Z

That actually suggests an unexpected option that doesn't involve new syntax: get(x, default).

JeffBezanson · 2018-05-30T19:17:29Z

I assume we would want ?? to evaluate its second argument lazily (in all cases)? If so, there would still be a need for a function version of it. I like get; I thought there might already be 1- or 2-argument methods of it but that doesn't seem to be the case. Also has a slight precedent in Scala's getOrElse. Its conciseness can't be beat and it is definitely similar to get on dicts.

I also have to admit this makes me entertain the heretical thought that we should consider combining nothing and missing. Even though they have a clear difference in meaning, the extra simplicity and syntax possibilities might be worth it.

StefanKarpinski · 2018-05-30T19:24:11Z

Yes, it would be lazy, of course, I should have indicated that. Having both nothing and missing has turned out to be more annoying than I had anticipated but it's a bit late in the game to change now.

JeffBezanson · 2018-05-30T19:42:38Z

True. The ? proposals could give us a clear division of labor: nothing gets various ? syntaxes, and missing gets 3VL and some common functions defined on it. Then we get to see if one ends up being much more useful, or if both are equally useful in different contexts.

nalimilan · 2018-05-30T19:49:37Z

It would totally make sense to use all the ? syntaxes for nothing. missing doesn't need them, it just needs another syntax akin to T?.

EDIT: We used get to unwrap Nullable, but it didn't really turn out to be a good idea IMHO. It's a quite different operation from getting a dict entry, where the second argument is the key. The only similarity is the presence of a default value.

JeffBezanson · 2018-05-31T19:43:22Z

My impression is that the main problem with get on Nullable was that people didn't want to unwrap the values at all, not that get was a bad name for it. Also, you can imagine generalizing get to multiple indices: get(dict, idxs..., default), where get(dict, default) is the 0-index case. Some would be acting like a 0-d container.

nalimilan · 2018-05-31T20:09:02Z

But then it's weird when not all arguments are Some, and some calls like get("text", 1, 1), get([1], 1, 1) and get(Dict(1=>2), 1, 1) are downright ambiguous.

vtjnash · 2018-05-31T20:12:04Z

get on Nullable was also a problem because it was slow, so we were adding crazy workarounds to deal with that. get with the Union approach does not suffer from that issue, and the workarounds are not needed

ref JuliaLang/julia#27258

* make `coalesce` handle only `missing`; add `something` to handle `nothing`/`Some` fixes #26927 * Move something docs with those for nothing and Some

JeffBezanson added the missing data Base.missing and related functionality label May 25, 2018

nalimilan reviewed May 26, 2018

View reviewed changes

nalimilan force-pushed the jb/missingsomething branch from 6caeb71 to 60d0141 Compare May 27, 2018 20:25

JeffBezanson and others added 2 commits May 28, 2018 08:50

make coalesce handle only missing; add something to handle `not…

1333048

…hing`/`Some` fixes #26927

Move something docs with those for nothing and Some

143c7f1

ViralBShah force-pushed the jb/missingsomething branch from 60d0141 to 143c7f1 Compare May 28, 2018 12:52

ViralBShah mentioned this pull request May 28, 2018

move statistics.jl and related methods from Base to StatsBase JuliaStats/StatsBase.jl#379

Merged

JeffBezanson added this to the 0.7 milestone May 28, 2018

ViralBShah merged commit 8aa54ac into master May 30, 2018

martinholters deleted the jb/missingsomething branch May 30, 2018 18:50

maleadt added a commit to JuliaGPU/CUDAnative.jl that referenced this pull request Jun 1, 2018

Replace coalesce with something.

f990da0

ref JuliaLang/julia#27258

odow mentioned this pull request Jul 7, 2018

something not defined jump-dev/MathOptInterface.jl#421

Closed

Keno pushed a commit that referenced this pull request Jun 5, 2024

make coalesce handle only missing; add something (#27258)

8f67442

* make `coalesce` handle only `missing`; add `something` to handle `nothing`/`Some` fixes #26927 * Move something docs with those for nothing and Some

Uh oh!

make coalesce handle only missing; add something #27258

make coalesce handle only missing; add something #27258

Uh oh!

Conversation

JeffBezanson commented May 25, 2018

Uh oh!

nalimilan left a comment

Choose a reason for hiding this comment

Uh oh!

nalimilan May 26, 2018

Choose a reason for hiding this comment

Uh oh!

StefanKarpinski May 26, 2018

Choose a reason for hiding this comment

Uh oh!

nalimilan May 26, 2018

Choose a reason for hiding this comment

Uh oh!

nalimilan May 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nalimilan May 26, 2018

Choose a reason for hiding this comment

Uh oh!

nalimilan May 26, 2018

Choose a reason for hiding this comment

Uh oh!

nalimilan May 26, 2018

Choose a reason for hiding this comment

Uh oh!

JeffBezanson May 26, 2018

Choose a reason for hiding this comment

Uh oh!

StefanKarpinski May 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nalimilan May 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nalimilan commented May 27, 2018

Uh oh!

nalimilan commented May 27, 2018

Uh oh!

ViralBShah commented May 27, 2018

Uh oh!

ViralBShah commented May 28, 2018

Uh oh!

ViralBShah commented May 28, 2018

Uh oh!

iblislin commented May 28, 2018

Uh oh!

ViralBShah commented May 28, 2018

Uh oh!

JeffBezanson commented May 28, 2018

Uh oh!

JeffBezanson commented May 28, 2018

Uh oh!

JeffBezanson commented May 28, 2018

Uh oh!

nalimilan commented May 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JeffBezanson commented May 28, 2018

Uh oh!

nalimilan commented May 28, 2018

Uh oh!

JeffBezanson commented May 30, 2018

Uh oh!

quinnj commented May 30, 2018

Uh oh!

nalimilan commented May 30, 2018

Uh oh!

ViralBShah commented May 30, 2018

Uh oh!

nalimilan commented May 30, 2018

Uh oh!

make `coalesce` handle only `missing`; add `something` #27258

make `coalesce` handle only `missing`; add `something` #27258

nalimilan May 26, 2018 •

edited

Loading

StefanKarpinski May 26, 2018 •

edited

Loading

nalimilan May 26, 2018 •

edited

Loading

nalimilan commented May 28, 2018 •

edited

Loading

yurivish commented May 30, 2018 •

edited

Loading

StefanKarpinski commented May 30, 2018 •

edited

Loading

nalimilan commented May 30, 2018 •

edited

Loading