-
Notifications
You must be signed in to change notification settings - Fork 70
Add transpose and transpose' to NonEmptyArray #228
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
Changes from 7 commits
7091ee9
634c870
9735c95
80ebea3
22e5396
950a4c9
38ecba0
0e8a530
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -397,6 +397,22 @@ testNonEmptyArray = do | |
log "traverse1 should work" | ||
assert $ traverse1 Just (fromArray [1, 2, 3, 4]) == NEA.fromArray [1, 2, 3, 4] | ||
|
||
log "transpose swaps rows and columns for a regular two-dimension array" | ||
assert $ NEA.transpose (fromArray [ (fromArray [1,2,3]), (fromArray [4,5,6]), (fromArray [7,8,9])]) == | ||
(fromArray [ (fromArray [1,4,7]), (fromArray [2,5,8]), (fromArray [3,6,9])]) | ||
|
||
log "transpose skips elements when rows don't match" | ||
assert $ NEA.transpose (fromArray [ (fromArray [10,11]), (fromArray [20]), (fromArray [30,31,32])]) == | ||
(fromArray [ (fromArray [10,20,30]), (fromArray [11,31]), (fromArray [32])]) | ||
|
||
log "transpose' swaps rows and columns for a regular two-dimension array" | ||
assert $ NEA.transpose' (fromArray [[1,2,3], [4,5,6], [7,8,9]]) == | ||
(fromArray [[1,4,7], [2,5,8], [3,6,9]]) | ||
|
||
log "transpose' skips elements when rows don't match" | ||
assert $ NEA.transpose' (fromArray [[10,11], [20], [30,31,32]]) == | ||
(fromArray [[10,20,30], [11,31], [32]]) | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can another test be added here for There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, this test fails - it produces transpose' :: forall a. NonEmptyArray (Array a) -> NonEmptyArray (Array a)
transpose' (NonEmptyArray [[]]) = NonEmptyArray [[]]
transpose' xs = (unsafeAdapt A.transpose) xs There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, I think my example is just wrong. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess the test should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Surely There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh gosh... 🤦♂️ Thanks! Then the implementation should be transpose' :: forall a. NonEmptyArray (Array a) -> Maybe (NonEmptyArray (Array a))
transpose' = fromArray <<< A.transpose <<< coerce There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not entirely happy with this. From my perspective, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The result of transposing |
||
odd :: Int -> Boolean | ||
odd n = n `mod` 2 /= zero | ||
|
||
|
Uh oh!
There was an error while loading. Please reload this page.