red / REP

Red Enhancement Process
BSD 3-Clause "New" or "Revised" License
11 stars 4 forks source link

WISH: `none` transparency for min/max #122

Closed hiiamboris closed 2 years ago

hiiamboris commented 2 years ago

I have encountered this need a lot across various projects. Got two values (e.g. size as pair, or alpha channel as tuple/4), one of them may be undefined (none). Always have to do special handling by hands. Instead it would be nice to have:

P.S. this also plays together with #113

greggirwin commented 2 years ago

My gut says having a separate, optional version is best (see below), since none is not comparable. If we do it for max/min, do we also change other comparisons to allow it?

; could also be called opt-max/opt-min

max?: func [
    value1 [scalar! series! none!] 
    value2 [scalar! series! none!] 
][
    any [
        all [value1 value2  max value1 value2]
        value1
        value2
    ]
]

min?: func [
    value1 [scalar! series! none!] 
    value2 [scalar! series! none!] 
][
    any [
        all [value1 value2  min value1 value2]
        value1
        value2
    ]
]
hiiamboris commented 2 years ago

Our implementations are the same ;)

min-safe: function [a [scalar! none!] b [scalar! none!]] [
    any [all [a b min a b] a b]
]
max-safe: function [a [scalar! none!] b [scalar! none!]] [
    any [all [a b max a b] a b]
]
hiiamboris commented 2 years ago

If we do it for max/min, do we also change other comparisons to allow it?

max/min are not related to comparisons, since they produce values not existing in the original set and not directly comparable:

>> max 2 1x3
== 2x3
>> max 1.2.3.4 3
== 3.3.3.4
>> max 2x1 0x3
== 2x3
greggirwin commented 2 years ago

I think they are related to comparisons. It's just that there are added semantics when you could reason about them. The tuple behavior is also new in Red. Can't think of a use case for it just now, considering the byte range limitation.

greggirwin commented 2 years ago

That is, to ask for min/max is a shortcut for using lesser/greater.

GiuseppeChillemi commented 2 years ago

Why not a /safe refinement here?

hiiamboris commented 2 years ago

A possibility

greggirwin commented 2 years ago

I vote No on /safe. We don't use it anywhere else, and the only relative is attempt/safer, which is about handling errors. None is not an error, so there is nothing to be "safe" from. Consider the widespread effect of explicit "could be none" semantics. Oh, wait, we have that today. :^)

What is wrong with the simple mezzanine approach here?

hiiamboris commented 2 years ago

What is wrong with the simple mezzanine approach here?

I'd rather ask what's wrong with none transparency being the default? That is, why should there be a separate mezz?

greggirwin commented 2 years ago

I know you would, but I asked you first. :^) Since I'm here though...

What's wrong with propagating none is that it makes errors less visible. It can be convenient at times, and Red has extended it to a number of cases. Now we're at the point where we need to think carefully about each use case and decide which behavior we think is best. It's the kind of thing that may break code later, no matter which way we go.

Your turn.

hiiamboris commented 2 years ago

Understandable concern, but my personal opinion here is that gain is bigger. Same as with remove and clear.

What I think we should worry about if it's a good match for HOFs or not? map :min series and none transparency. But I have no arguments in this matter yet.

Your turn.

min/max are great names and I don't want to uglify them with what we have above. Performance matters too, though so far min/max weren't the critical parts for me personally (but I can imagine cases where they are).

greggirwin commented 2 years ago

Thanks for your thoughts. I still vote No. We can always change it later, people can use a mezz now, which will also tell us how many people do (need that darn module system, and maybe one that works at the granular function level). If we make it standard later, yes, they'll have to update their code if they choose to use the native version, but nothing breaks.

GiuseppeChillemi commented 2 years ago

Note on closed ticked: we have talked about none transparency for index? too, for the phrase index? find series value. The ending was the same: we keep none for "security" but I see an opening to take a look again in the future.

dockimbel commented 2 years ago

Got two values (e.g. size as pair, or alpha channel as tuple/4), one of them may be undefined (none).

Could you give a concrete code example(s)?

What is wrong with using any [<expr-that-can-return-none> <default-value>] code idiom?

I'd rather ask what's wrong with none transparency being the default? That is, why should there be a separate mezz?

None-transparency means removing some type-checking, which is making the code less robust. We already have a very dynamic language. If we remove the little safety barriers we have, we are making it harder for all users to track bugs (as the bug would have consequences (much) later in the code execution path, leading to the most expensive kind of bugs to track).

So, I'm generally against none-transparency, though some specific code patterns seem to benefit more than suffer from it. It's still a very slippery slope, so I would only allow it on code patterns where the gains are obvious to everyone. Still a pretty subjective case-by-case decision, so I would prefer we don't spread it furthermore.