New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Coerce set comparator operands 'up' #335
Conversation
This patch presents the position that comparator ops could be more
DWIM-y by always coercing their arguments up the chain of complexity.
The current behavior is to coerce Setty things to Sets. This puts Bags
and Mixes in an awkward situation, as .values become True and so only
.keys comparisons matter. The approach of having (<+) and the like be
Baggy specific versions of the operator smells overcomplicated to me.
This approach preserves the original behavior by asking our dev to
be explicit about any "downgrading" of any .values inasmuch as they are
present in the operation:
$set (<) $bag.Set
This demand to a more explicit statement is the only perceived
tradeoff*, the reward of which is that one does not need to think twice
about:
my $reference-bag = bag <1 1 1 2 2 3>;
my $random-mix = mix <1 2 2 3 3 3>;
# Existing syntax:
# Bag --> Set (<=) Mix --> Set
> $reference-bag (<=) $random-mix
True
# Coercion of least surprise:
# Bag --> Mix (<=) Mix
> $reference-bag (<=) $random-mix
False
There is another option to implement this, using multi methods and
signatures. However, this implementation is way DRY-er, and thus a
better example of code (which I take it the core settings are intended
to be.. and that's judging mainly from the looks of things!).
Notes:
*Which is also arguably a better demand to make than to ask our dev to
remember a separate-but-strange (<+))
|
Indeed, the patch is intended to open a dialog. Fair points regarding justification for the patch. The crux of the idea is that these violate principle of least surprise. Considering that other set operators now DWIM by considering the Bagginess of their arguments, it seems kludgy to have the comparators not give our dev the same respect. And certainly the idea would be to deprecate the separate subbag operators, were core to adopt the position that this patch proposes. I also don't have time for more now, but am glad the discussion is open. |
|
This patch does propose more than it contains, and so I apologize for being too Right now our set operators are all but useless for Mixes. They are either This is perhaps reflected in the fact that there are no tests for mix operators. The solution taken so far (but only with the comparators) is to add a duplicate So either we need to triplicate each operator (one each for set, mix, and bag) So, this patch is not ready to be merged because it is only here to start the Additional note: there are some operators that need to be modified to not use |
|
I feel like in some ways this is in the realm of argument-dependent vs argument-coercive operations like PHP-type TL;DR: In my view: the set-specific operators are good because they clarify semantics; the other ops should probably coerce to the most general type provided. |
|
Perhaps the appropriate way to start would be to tackle getting Mix to work
|
|
colomon, that is a bigger job and so I was hoping to have some clarity on what the expectations are for set operators before I go there. Mouq, where do you draw the line for 'set specific' operators? Is it based on which ones currently have In my opinion, the proposal is less about The main thing here is that this is one of the places where an explanation has to be delivered, either way. This explanation should be as easy as possible. Pointing to a line in the sand somewhere doesn't really meet that criteria. If set operators should only do sets, then I think that we should have a dedicated Then the choice is between whether the explanation should be 'use this for QuantHash' ( (Edited for tone, intent, and mistakes). |
|
I have some tests available over here: ab5tract/roast@708cf5d |
|
i believe we have something like this in rakudo now, though i forgot who exactly implemented what we have now. @lizmat, i think you touched that code a bunch? |
|
All of the set operators have been almost completely reimplemented since then. They now occupy several source files, with prefix The principle is that neither side of a set operator needs to be a Furthermore, bag and mix are contagious: as soon as one of the parameters of a set operator is a Closing this now. |
This patch presents the position that comparator ops could be more
DWIM-y by always coercing their arguments up the chain of complexity.
The current behavior is to coerce Setty things to Sets. This puts Bags
and Mixes in an awkward situation, as .values become True and so only
.keys comparisons matter. The approach of having (<+) and the like be
Baggy specific versions of the operator smells overcomplicated to me.
This approach preserves the original behavior by asking our dev to
be explicit about any "downgrading" of any .values inasmuch as they are
present in the operation:
This demand to a more explicit statement is the only perceived
tradeoff*, the reward of which is that one does not need to think twice
about:
There is another option to implement this, using multi methods and
signatures. However, this implementation is way DRY-er, and thus a
better example of code (which I take it the core settings are intended
to be.. and that's judging mainly from the looks of things!).
Notes:
*Which is also arguably a better demand to make than to ask our dev to
remember a separate-but-strange (<+))