Skip to content
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

List.transpose discards data instead of throwing an exception #6908

Closed
ntwilson opened this issue May 30, 2019 · 3 comments · Fixed by #6989
Closed

List.transpose discards data instead of throwing an exception #6908

ntwilson opened this issue May 30, 2019 · 3 comments · Fixed by #6989
Labels
Area-Library Issues for FSharp.Core not covered elsewhere Bug
Milestone

Comments

@ntwilson
Copy link

ntwilson commented May 30, 2019

When List.transpose is given a list containing an empty sublist, all sublists after the empty sublist are discarded.
It's easier to explain by example:

> List.transpose [[true; true]; []; [false; false]];;
val it : bool list list = [[true]; [true]]

compared to:

> Array.transpose [| [|true; true|]; [||]; [|false; false|] |];;
System.ArgumentException: The arrays have different lengths.
array.[0].Length = 2, array.[1].Length = 0
Parameter name: array.[0]
   at Microsoft.FSharp.Core.DetailedExceptions.invalidArgDifferentArrayLength[?](String arg1, Int32 len1, String arg2, Int32 len2)
   at Microsoft.FSharp.Collections.ArrayModule.transposeArrays[T](T[][] array)
   at Microsoft.FSharp.Collections.ArrayModule.Transpose[T](IEnumerable`1 arrays)
   at <StartupCode$FSI_0018>.$FSI_0018.main@()
Stopped due to error

And:

> Seq.transpose [[true; true]; []; [false; false]];;
val it : seq<seq<bool>> = seq [seq [true; false]; seq [true; false]]

though the Seq.transpose behavior is understandably quite different.

Provide the steps required to reproduce the problem:

  1. open FSI
  2. enter List.transpose [[true; true]; []; [false; false]];;

Expected behavior

I would expect it to throw an exception just like the Array.transpose implementation, or maybe just filter out empty sublists, such that List.transpose [[true; true]; []; [false; false]] would return [[true; false]; [true; false]]

Actual behavior

List.transpose discards any sublists after the first empty sublist (from what I can tell). For the particular case of List.transpose [[true; true]; []; [false; false]] it returns [[true]; [true]]

Known workarounds

  1. check that all the sublists have the same length before calling transpose, and handling the error case directly.
  2. convert to a seq of arrays and use Array.transpose

Related information

  • Windows 10
  • FSI shipped with Visual Studio 2019 16.1.0, particularly
    C:\Program Files (x86)\Microsoft Visual Studio\2019\Professional\Common7\IDE\CommonExtensions\Microsoft\FSharp\fsi.exe
@cartermp
Copy link
Contributor

cartermp commented Jun 3, 2019

I agree that the behavior isn't correct. @PatrickMcDonald any insights or do you concur?

FWIW I could see this going either direction, but given that we don't throw with lists to ease with composition scenarios (see creation of empty ranges or ranges with negative indices) I think it would be more expected to give [[true; false]; [true; false]] as a result.

I'd be happy to accept a PR that changes this behavior if there's no objections.

@cartermp cartermp added Area-Library Issues for FSharp.Core not covered elsewhere Bug labels Jun 3, 2019
@cartermp cartermp added this to the Backlog milestone Jun 3, 2019
@cartermp
Copy link
Contributor

cartermp commented Jun 3, 2019

Also worth noting that it is a breaking change to adjust List to have any changed behavior here, but given that it simply doesn't transpose anything I'd argue that it's worth changing.

@PatrickMcDonald
Copy link
Contributor

PatrickMcDonald commented Jun 5, 2019

I agree this is a bug, and we should probably throw

Note that this situation only occurs when an empty list is passed, the following will throw an exception

> List.transpose [[true; true]; [false]; [false; false]];;
System.ArgumentException: The lists had different lengths.
list.[1] is 1 element shorter than list.[0]
Parameter name: list.[1]
   at Microsoft.FSharp.Core.DetailedExceptions.invalidArgDifferentListLength[?](String arg1, String arg2, Int32 diff)
   at Microsoft.FSharp.Primitives.Basics.List.transposeToFreshConsTail[a](FSharpList`1 cons, FSharpList`1 list, Int32 expectedCount)
   at Microsoft.FSharp.Primitives.Basics.List.transpose[T](FSharpList`1 list)
   at Microsoft.FSharp.Collections.ListModule.Transpose[T](IEnumerable`1 lists)
   at <StartupCode$FSI_0008>.$FSI_0008.main@()
Stopped due to error

PatrickMcDonald added a commit to PatrickMcDonald/visualfsharp that referenced this issue Jun 12, 2019
* transpose does not throw when one of the elements is empty
TIHan pushed a commit that referenced this issue Jun 27, 2019
)

* List.transpose should throw error when given jagged array (#6908)

* transpose does not throw when one of the elements is empty

* Add additional test cases
@cartermp cartermp modified the milestones: Backlog, 16.3 Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Library Issues for FSharp.Core not covered elsewhere Bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants