New PlainListCopy vs. AsPlist: unify?

[fingolfin]

In PR #4332, the GAP library function PlainListCopy (which has existed for many years but was undocumented) was fixed and enhanced, for use in PR #4331.

Today I came upon the GAP library operation AsPlist which seems to serve a very similar purpose, with one key difference: if the input is already a plain list, it just returns that, i.e., not a copy. That's also what the documentation of that function says (but note that it is not documented in the GAP reference manual, i.e., it is "undocumented" for purposes of whether we made any binding promises about it or not). Also, no package I have access to uses AsPlist

Note we already have a bunch of "pairs" like List/AsList, Set/AsSet; typically, the first function returns a mutable object, the As* version an immutable one. Based on that, it is also strange that AsPlist returns something mutable...

How about the following:

• rename PlainListCopy to Plist (and/or have one be a synonym of the other)
• change AsPlist to return an immutable copy instead
• for efficiency reasons (dispatch where an argument is a list is slow), perhaps AsPlist should be a kernel operation or a
• simple global function which only dispatches to an actual operation, say AsPlistOp)

On the other hand, it may be useful for efficiency reasons to have a function like the current AsPlist, i.e., which returns the original input if it was a plist all along. But looking at the GAP library, I don't really think it matters much if at all to the existing use cases, which mostly look like this: AsSSortedListList(AsPlist(xxx)). So yeah, if xxx is plain list, we save a bit, but typically the relevant methods already have a special case for IsPlistRep, e.g.

InstallOtherMethod(AsSSortedList,
    "for a plist",
    [IsList and IsPlistRep],
    AsSSortedListList );

InstallOtherMethod(AsSSortedList,
     "for a list",
     [ IsList ],
     l -> AsSSortedListList( AsPlist( l ) ) );

Thoughts? It would be good to resolve this before the GAP 4.12.0 release, so that we never announce PlainListCopy if we decide to rename it.

[ChrisJefferson]

I would support moving to renaming PlainListCopy to AsPlist (and throwing away the current AsPlist implementation). I don't really like functions that might, or might not, return a copy, I'd prefer it people really don't want a copy they explicitly check (as with the AsSSortedList example you give).

[fingolfin]

But I was not suggesting to rename PlainListCopy to AsPlist, as I think AsPlist is also badly named -- other AsFOO functions return something immutable.

I guess I am suggesting several separate things:

1. to merge PlainListCopy to AsPlist into one function.
2. to not keep the name AsPlist as it is inconsistent.
3. to actually call the new function Plist.

But thinking about point 3 again, that mostly makes sense if we also want to have an AsPlist version returns an immutable plist... otherwise, perhaps PlainListCopy or PlainList is better? I dunno -- but what I care about more are points 1 and 2 anyway. So e.g. we could retain the name PlainListCopy and just replace AsPlist by it...

[ChrisJefferson]

Sorry, I should have read your message more carefully.

I think we should aim for consistency, so having both Plist, and AsPlist, seems sensible.