Move mutating functions in the Array module to a new Array.InPlace / Array.Mutating

[Happypig375]

Move mutating functions in the Array module to a new Array.InPlace / Array.Mutating

I propose we deprecate:

Array.blit: source:'T[] -> sourceIndex:int -> target:'T[] -> targetIndex:int -> count:int -> unit
Array.fill: target:'T[] -> targetIndex:int -> count:int -> value:'T -> unit
Array.set: array:'T[] -> index:int -> value:'T -> unit
Array.sortInPlace: array:'T[] -> unit when 'T : comparison 
Array.sortInPlaceBy: projection:('T -> 'Key) -> array:'T[] -> unit when 'Key : comparison 
Array.sortInPlaceWith: comparer:('T -> 'T -> int) -> array:'T[] -> unit

and add (using InPlace as an example)

Array.InPlace.copyFrom: sourceIndex:int -> targetIndex:int -> count:int -> source:'T[] -> target:'T[] -> 'T[]
Array.InPlace.fillAt: index:int -> count:int -> value:'T -> array:'T[] -> 'T[]
Array.InPlace.updateAt: index:int -> value:'T -> array:'T[] -> 'T[]
Array.InPlace.sort: array:'T[] -> 'T[] when 'T : comparison 
Array.InPlace.sortBy: projection:('T -> 'Key) -> array:'T[] -> 'T[] when 'Key : comparison 
Array.InPlace.sortWith: comparer:('T -> 'T -> int) -> array:'T[] -> 'T[]

We can also consider adding

Array.InPlace.map : mapping:('T -> 'T) -> array:'T[] -> 'T[]
Array.InPlace.mapi: mapping:(int -> 'T -> 'T) -> array:'T[] -> 'T[]
Array.InPlace.permute: indexMap:(int -> int) -> array:'T[] -> 'T[]
Array.InPlace.rev: array:'T[] -> 'T[]
Array.InPlace.sortDescending: array:'T[] -> 'T[] when 'T : comparison
Array.InPlace.sortByDescending: projection:('T -> 'Key) -> array:'T[] -> 'T[] when 'Key : comparison
Array.InPlace.fill: value:'T -> array:'T[] -> 'T[]
Array.fillAt: index:int -> count:int -> value:'T -> array:'T[] -> 'T[]
Array.fill: value:'T -> array:'T[] -> 'T[]
Array.InPlace.shuffle: array:'T[] -> 'T[] // Given https://github.com/fsharp/fslang-suggestions/issues/508

and deprecating

Array.get: array:'T[] -> index:int -> 'T // Use Array.item instead, since symmetry with Array.set is gone, and Array.item is consistent with List and Seq functions

in the process.

The existing way of approaching this problem in F# is manually checking which function mutates among lots of copying functions in the same module.

Pros and Cons

The advantages of making this adjustment to F# are

1. Making mutating and copying functions distinctive
2. Easier optimisation of existing functions just by adding a .InPlace / .Mutating
3. Now we can pipe mutating operations instead of requiring a new let binding. It reads fluently and is comparable to fluent dotted calls.

The disadvantages of making this adjustment to F# are

1. For piping to work, these new functions return the input mutated array instead of unit. This makes the unit-returning = mutating harder to follow. However, existing C# fluent dotted calls already do this.
2. There are changes to the function names and argument orders to match the non-mutating counterparts. However, we also achieve consistency here.
3. Additions of more mutating functions may encourage mutable programming. However, just like Array.Parallel, these are only here for optimisation and you should always default to the normal Array functions. With Array.Parallel's precedence, the risk of encouraging mutable programming is minimised.

Extra information

Estimated cost (XS, S, M, L, XL, XXL): M to L

Related suggestions: #508

Affidavit (please submit!)

Please tick this by placing a cross in the box:

• This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
• I have searched both open and closed suggestions on this site and believe this is not a duplicate
• This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

• This is not a breaking change to the F# language design
• I or my company would be willing to help implement and/or test this

For Readers

If you would like to see this issue implemented, please click the 👍 emoji on this issue. These counts are used to generally order the suggestions by engagement.

[cartermp]

The disadvantages of making this adjustment to F# are

The biggest disadvantage is that this is a sweeping breaking change :)

[Happypig375]

Also this is in line with the Theme of "Simple F#": https://github.com/dotnet/fsharp/issues?q=label%3ATheme-Simple-F%23+

[Happypig375]

The biggest disadvantage is that this is a sweeping breaking change :)

It's as breaking as deprecating nth to force usage of item, := and ! to force usage of .Value etc.

[cartermp]

It's as breaking as

I'm not aware of nth, but the latter category you're describing is not on the docket. Those are deprecation warnings with tooling proposed to be implemented to make changes to code. You've not called this out since the title is to move these functions elsewhere. That is a sweeping breaking change.

[Happypig375]

You've not called this out since the title is to move these functions elsewhere.

The title did not use "deprecate" then "add" since it will be too long. The first sentence in the description is:

I propose we deprecate:

[laenas]

At a bare minimum, wouldn't it make more sense to allow optional opening of a module that shadows standard core behavior with mutable defaults?
So that existing code sees no change of behavior, for better or worse, but with the introduction of open FSharp.Core.Array.InPlace - all the usual functions exist from an API surface perspective, but have inplace/mutable semantics?

[laenas]

To be clear: the renaming also concerns me, since that breaks swaths of code, and love it or not, we have blit etc now.

For cases like sort - we'd end up with default behavior being copy as-is, sortInPlace being available as-is today, but upon opening the shadowing-namespace, you'd have sort being equivalent to sortInPlace which would still be available anyway.

[Happypig375]

At a bare minimum, wouldn't it make more sense to allow optional opening of a module that shadows standard core behavior with mutable defaults?

Not possible because array resizing / type-changing in-place and returning an array type requires additional allocations. We can make use of ArraySegment / Span / ReadOnlySpan, but piping is gone and additional modules are required.

[cartermp]

The title did not use "deprecate" then "add" since it will be too long. The first sentence in the description is:

Okay, well we can argue this to death and ensure nothing happens here, but as it stands I don't support this.

[bisen2]

The function signatures already serve to distinguish mutating functions from non-mutating. With the proposed changes we trade a function signature that clarifies that a function is mutating for the assumption that all functions within a certain module are mutating. Personally I would prefer to rely on the type signatures than an assumption that all functions in a module conform to an idea the module is named after.

[Happypig375]

To be clear: the renaming also concerns me, since that breaks swaths of code, and love it or not, we have blit etc now.

And they are confusing to new users because they sit alongside copying functions that are to be expected in other collection modules.

[Happypig375]

Personally I would prefer to rely on the type signatures than an assumption that all functions in a module conform to an idea the module is named after.

You already lose unit returns = mutations rule upon interop with the outside like .NET or JS as in Fable.

[voronoipotato]

What if instead there was an attribute which allows you to easily see which functions in the standard lib mutate or may mutate? It often makes sense to both return something AND mutate, but it would be nice if we had a signal to the consumer of that function that we could use to let them know "Hey this mutates". A function that isn't tagged may or may not mutate, but it would be nice to explicitly know that some do mutate. Perhaps you could even scan to look for functions which call functions/methods that return unit and generate the annotation.

I do agree with @cartermp that this as proposed a big breaking change that will upset people regardless of how good it is, because some people have massive codebases and they'll reasonably interpret "deprecated" as "change it".

Being said your Array.InPlace might be a good nuget package to make.

[uxsoft]

When I was starting with F# I was introduced to Array and List as immutable collections. When I first saw a method that mutated the array I was totally confused. So is it immutable or not? How can I now trust that it's immutable when I can easily mutate it.

I like moving these to a separate module because it clearly shows that you're mutating the array at first glance and that those should only be used if you know what you're doing.

Also personally I don't really have a problem with the deprecation. Especially if we can have a quick-action and one-click apply it in the whole solution to change everything.

[laenas]

When I was starting with F# I was introduced to Array and List as immutable collections.

I'd be curious where - since the langref is clear about cell-mutability. This is behavior going back 4 decades or more in the ML family of languages - so any source citing that arrays are immutable is suspect.

[uxsoft]

So the story could be very similar to this article: https://www.compositional-it.com/news-blog/immutable-data-structures-in-f/

People would talk about why F# is cool and why immutable collections are the way to go and then they use Array. Maybe not explicitly stating that array is immutable but I hope you can see how somebody who didn't read the manual might assume.

There's also the part where most of the time you'd use methods that don't mutate the array so you're not exposed to the mutability aspect of Array much.

Anyway, Just trying to highlight the positives of this change. In my humble opinion, it does make the behavior of the functions a bit more explicit and organized which might help some people.

[Happypig375]

The Array module should stay immutable, while the mutability of arrays themselves are too prevalent to change. We should stay to the module functions to guarantee immutability.

[yatli]

How about ImmutableArray?

[Happypig375]

@yatli That's #619 territory. ImmutableArray refers to System.Collections.Immutable.ImmutableArray.

[dsyme]

I agree with comments above that it's too much of a breaking change all in all

As a result I will close this

[Happypig375]

How come Option.get is to be deprecated but not mutating array functions?

[uxsoft]

How come Option.get is to be deprecated but not mutating array functions?

I understood from this that the agreed solution to deprecating mutating array functions was to introduce a new immutable array block and keep the array mutable as is because otherwise, it would be a big breaking change.

More on block here: #619

[Happypig375]

like any mutations can still go through the indexer as well as the new Array.InPlace module. Mutating is still possible but we can trust Array module functions to be immutable.

[Tarmil]

How come Option.get is to be deprecated but not mutating array functions?

Because throwing an exception and mutating aren't the same thing? 😐