[PATCH] misc cleanup: use 'concat' instead of '"append"/'

[oldk1331]

I don't like the grammar that uses

    "func"/list1

to do

    reduce(func, list1)


First, this is a strange grammar.

Also I don't like to have special grammar to do things can
be done by normal grammar.

This patch replaces '"append"/' with normal function call
'concat : List % -> %', it's simpler and faster.

If you agree, I plan to replace other usage of '"func"/' by
normal function calls.

grep '"[a-zA-Z]*" */' *spad   shows 135 usage of such grammar,
only uses function max/min/setUnion/and/or/gcd/lcm.

[Ralf Hemmecke]

Hi Qian,

I support such a change, in fact, I don't like something like

"+" / list

because it's not exactly clear what happens for the empty list.
I'd rather always put the three-argument "reduce" in order to specify
the value that is to be returned for the empty list.

Note that
http://fricas.github.io/api/Collection.html#l-collection-reduce
says:


reduce: ((S, S) -> S, %) -> S if % has finiteAggregate
reduce(f, u) reduces the binary operation f across u.
For example, if u is [x, y, ..., z] then reduce(f, u)
returns f(..f(f(x, y), ...), z). Note: if u has one
element x, reduce(f, u) returns x.
Error: if u is empty.
^^^^^^^^^^^^^^^^^^^^^

And please include "-+*" in your regular expression.
Furthermore there are also cases like

n := _+/[#a for a in l]

for example in aggcat.spad.

Ralf

[Waldek Hebisch]

oldk1331 wrote::

>
> I don't like the grammar that uses
>
> "func"/list1
>
> to do
>
> reduce(func, list1)
>
>
> First, this is a strange grammar.
>
> Also I don't like to have special grammar to do things can
> be done by normal grammar.

I am affraid the classic answer is appropriate here
"it is a feature, not a bug".

Expanding on it a bit: it is usual that syntactic choices
cause some disagreement. From my point of view this is
useful shortcut for frequently occuring operation.
It does not significantly increase complexity of the
parser and beside parser we need smal amount of extra
code. So from implementation point of view it is
cheap feature. I so no reason to remove it.

@Ralf: this construct kowns about identity for several
popular operations, so in such case works also for empty
lists. Of course, it would be nicer to do this in more
systematic way.

>
> This patch replaces '"append"/' with normal function call
> 'concat : List % -> %', it's simpler and faster.
>

Well, simpler is debatable. Faster is useful if speed
matters.

>
> If you agree, I plan to replace other usage of '"func"/' by
> normal function calls.
>
> grep '"[a-zA-Z]*" */' *spad shows 135 usage of such grammar,
> only uses function max/min/setUnion/and/or/gcd/lcm.

Well, as I wrote I think that "op"/... is a useful feature.
And it is useful to have some test cases for features.
So I would like to keep some (preferaby most) of uses of
this construct.

--
Waldek Hebisch

[oldk1331]

On 8/10/19 2:17 AM, Waldek Hebisch wrote:
> oldk1331 wrote::
>>
>> I don't like the grammar that uses
>>
>> "func"/list1
>>
>> to do
>>
>> reduce(func, list1)
>>
>>
>> First, this is a strange grammar.
>>
>> Also I don't like to have special grammar to do things can
>> be done by normal grammar.
>
> I am affraid the classic answer is appropriate here
> "it is a feature, not a bug".
>
> Expanding on it a bit: it is usual that syntactic choices
> cause some disagreement. From my point of view this is
> useful shortcut for frequently occuring operation.

We already have an useful shortcut for this frequently
occurring pattern (the reduce pattern): function "reduce".

> It does not significantly increase complexity of the
> parser and beside parser we need smal amount of extra
> code. So from implementation point of view it is
> cheap feature. I so no reason to remove it.

I think it's bad design to have special grammar do things
that normal function can do.

> @Ralf: this construct kowns about identity for several
> popular operations, so in such case works also for empty
> lists. Of course, it would be nicer to do this in more
> systematic way.

It's also a bad design to hard-code extra knowledge into
compiler.

>> This patch replaces '"append"/' with normal function call
>> 'concat : List % -> %', it's simpler and faster.
>>
>
> Well, simpler is debatable. Faster is useful if speed
> matters.

For common usage, we already have "gcd : List(%) -> %"
defined as "gcd(l : List %) == reduce(gcd, l, 0, 1)".
now comparing the following:

"gcd"/[f(i) for i in l]
gcd [f(i) for i in l]

At least it is 3 characters simpler and less confusing.

>>
>> If you agree, I plan to replace other usage of '"func"/' by
>> normal function calls.
>>
>> grep '"[a-zA-Z]*" */' *spad shows 135 usage of such grammar,
>> only uses function max/min/setUnion/and/or/gcd/lcm.
>
> Well, as I wrote I think that "op"/... is a useful feature.
> And it is useful to have some test cases for features.
> So I would like to keep some (preferaby most) of uses of
> this construct.
>

I propose I only do the cleanup for "concat", "gcd" , "lcm"
for now, because they already have signature "List % -> %".

[Ralf Hemmecke]

On 8/15/19 5:03 AM, oldk1331 wrote:
> On 8/10/19 2:17 AM, Waldek Hebisch wrote:
>> oldk1331 wrote::
>>>
>>> I don't like the grammar that uses
>>>
>>> "func"/list1
>>>
>>> to do
>>>
>>> reduce(func, list1)
>>>
>>>
>>> First, this is a strange grammar.
>>>
>>> Also I don't like to have special grammar to do things can
>>> be done by normal grammar.
>>
>> I am affraid the classic answer is appropriate here
>> "it is a feature, not a bug".
>>
>> Expanding on it a bit: it is usual that syntactic choices
>> cause some disagreement. From my point of view this is
>> useful shortcut for frequently occuring operation.
>
> We already have an useful shortcut for this frequently
> occurring pattern (the reduce pattern): function "reduce".
>
>> It does not significantly increase complexity of the
>> parser and beside parser we need smal amount of extra
>> code. So from implementation point of view it is
>> cheap feature. I so no reason to remove it.
>
> I think it's bad design to have special grammar do things
> that normal function can do.

I agree with Qian on this.

Suppose in a domain I have a domain that exports

+: (%, %) -> %

and I define a function

/: ((%, %) -> %, List %) -> %

and then write

_+ / [a, b, c]

for a, b, c of type %. Will the compiler use its internal knowledge
about the "x" / list special grammar or will it use my definition of / ?

> For common usage, we already have "gcd : List(%) -> %"
> defined as "gcd(l : List %) == reduce(gcd, l, 0, 1)".
> now comparing the following:
>
> "gcd"/[f(i) for i in l]
> gcd [f(i) for i in l]
>
> At least it is 3 characters simpler and less confusing.

I support the call without "/".

> I propose I only do the cleanup for "concat", "gcd" , "lcm"
> for now, because they already have signature "List % -> %".

This would also be a compromise for now. Eventually, I also think, we
should remove this special grammar.

Ralf

[Waldek Hebisch]

Ralf Hemmecke wrote:
>
> Suppose in a domain I have a domain that exports
>
> +: (%, %) -> %
>
> and I define a function
>
> /: ((%, %) -> %, List %) -> %
>
> and then write
>
> _+ / [a, b, c]
>
> for a, b, c of type %. Will the compiler use its internal knowledge
> about the "x" / list special grammar or will it use my definition of / ?

As written this will work as normal overloaded operator.

The special syntax requires string form, that is

"+" / [a, b, c]

If you use string form, then this will be transformed regardless
of other functions that you define.

--
Waldek Hebisch

[Ralf Hemmecke]

Oh. OK. Then we should get rid of the string from of an operator. I
actually find usage of "+" as an operator quite strange, since it pretty
much looks like a string. I can live for now with _+, but just using the
operator directly, for example, in sort(<, list) would look a bit nicer.

Ralf