Wrong argument of DeclareAttribute in SONATA package

[olexandr-konovalov]

Now happening in the master branch:

 ┌───────┐   GAP 4.8.8-4974-g3afc42f of today
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-apple-darwin16.7.0-default64
 Configuration:  gmp 6.1.2, readline
 Loading the library and packages ...
 Packages:   GAPDoc 1.6.1, PrimGrp 3.3.0, smallgrp 1.2, TransGrp 2.0.2
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'
gap> LoadPackage("sonata");
Error, Usage: DeclareAttribute( <name>, <filter>[, <mutable>][, <rank>] ) at /Users/alexk/GITREPS/gap/lib/oper.g:1285 called from
<function "DeclareAttribute">( <arguments> )
 called from read-eval loop at /Users/alexk/GITREPS/gap/pkg/sonata/lib/nr.gd:324
you can 'quit;' to quit to outer loop, or
you can 'return;' to continue
brk> 

The line in question in SONATA is

DeclareAttribute( "NRRowEndos", IsNearRing, IsMutable );

[fingolfin]

This is clearly a bug in SONATA

[olexandr-konovalov]

Oh yes, I see - documentation requires

DeclareAttribute( name, filter[, "mutable"][, rank] )

I will let SONATA authors know. That now may break tests for some packages where SONATA appears in dependencies (e.g. Wedderga through GUAVA).

P.S. Seems to be revealed by #2078

[fingolfin]

This also look as if the SONATA package authors intended for NRRowEndos to be a mutable attribute... But it never was... So it is unclear to me (without inspecting the SONATA code) whether this can be a regular immutable attribute, or whether perhaps it really should become mutable now (but then: why did nobody ever notice it wasn't mutable?)

On a meta-level, mutable attributes really are an abomination, and also kind of violate the whole idea of "all subobjects of an immutable object are immutable".

[olexandr-konovalov]

While reporting this to SONATA author, I've checked for other cases like this. SONATA has some more calls of DeclareAttribute where the 3rd argument is "mutable" as required by documentation. But I don't know anything about the reasons behind this...

[olexandr-konovalov]

I have just send an email to Peter Mayr to remind about this.

[olexandr-konovalov]

Fix submitted in gap-packages/sonata#6. Although the question to @PMayr whether it's really necessary to use a mutable attribute there remains open.

[PMayr]

Looking at the SONATA code, I don't see why the attribute NRRowEndos should be mutable. In fact it is not used anywhere in SONATA. I don't know whether it is required for anything outside of SONATA, but if not we could just get rid of it.

[fingolfin]

Indeed, and I can't find any reference to NRRowEndos in any other package either. So just removing it seems indeed like the best option.

[olexandr-konovalov]

Fix merged in SONATA - waiting for the release to be published.

[fingolfin]

@alex-konovalov SONATA 2.9 was released 2018-10-01, and AFAIK fixes this. OK to close this now?

[olexandr-konovalov]

Yes - the new SONATA release now appears in bootstrapping package archives, and fixes this.