Add transpose and transpose' to NonEmptyArray

[newlandsvalley]

Description of the change

Now that we have transpose in Array we can add two variants of it to NonEmptyArray. transpose handles the case where both the inner and outer arrays are non-empty. transpose' handles the case where the inner arrays are typed as a standard Array. The transpose' implementation courtesy of @JordanMartinez .

For a discussion see #227.

---

Checklist:

• Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0000)")
• Linked any existing issues or proposals that this pull request should close
• Updated or added relevant documentation
• Added a test for the contribution (if applicable)

[JordanMartinez]

My comment below also applies to unsafeFromArray2D.

[JordanMartinez]

I think adding one more test should be done, but otherwise this looks good to go.

[JordanMartinez]

Can another test be added here for NEA.transpose' (fromArray [[]]) == (fromArray [[]])?

[newlandsvalley]

Hmm, this test fails - it produces (fromArray []). Perhaps we need to special-case it? This works:

transpose' :: forall a. NonEmptyArray (Array a) -> NonEmptyArray (Array a)
transpose' (NonEmptyArray [[]]) = NonEmptyArray [[]]
transpose' xs = (unsafeAdapt A.transpose) xs

[JordanMartinez]

Oh, I think my example is just wrong. Array has the same behavior in its test: https://github.com/purescript/purescript-arrays/blob/master/test/Test/Data/Array.purs#L280

[JordanMartinez]

I guess the test should be NEA.transpose' (fromArray [[]]) == (fromArray [])

[newlandsvalley]

Surely fromArray [] is an empty NonEmptyArray and thus eminently unsafe.

[JordanMartinez]

Oh gosh... 🤦‍♂️ Thanks!

Then the implementation should be

transpose' :: forall a. NonEmptyArray (Array a) -> Maybe (NonEmptyArray (Array a))
transpose' = fromArray <<< A.transpose <<< coerce

[newlandsvalley]

I'm not entirely happy with this. From my perspective, transpose' (fromArray [[]]) is a very odd thing to do anyway and I wouldn't have thought it would be entirely incorrect for it to be an idempotent operation. But the cost of doing what you suggest is to make all the normal cases more awkward by wrapping the result in the Maybe.

[JordanMartinez]

Awkward or not, it is correct. And if that's the definition, then it's the same as:

fromArray <<< A.transpose <<< NEA.toArray

So, I would argue now that we shouldn't implement that here.

[newlandsvalley]

I'll happily do what you suggest if it is the correct definition. Unless I hear otherwise from @garyb today I'll do it this way - I don't feel strongly about it in any case.

(However just to expand my approach a bit, I'd argue that the minimal result for transpose Array (Array a) is [], that for NonEmptyArray (NonEmptyArray a) is [[something]] and that for NonEmptyArray (Array a) is [[]]. There seems to be no equivalent function on ```NonEmptyList```` in the Haskell library. which might indicate correctness.)

[garyb]

The result of transposing [[]] being [[]] does sound pretty reasonable to me (in both non-or-empty cases), but I guess if we have evidence from elsewhere that that's not normal, then yeah having transpose' fail with Nothing is probably the best path forward.

[garyb]

I'll merge and release this later today, writing myself a script to make the process easier, and this will be the test case 😉

[garyb]

Released as v7.1.0