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.

Sign up for GitHub

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

Use Natural Transformations wherever possible #207

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Use Natural Transformations wherever possible #207

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Data/Array.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -628,7 +628,7 @@ intersperse a arr = case length arr of
-- | reverse [1, 2, 3] = [3, 2, 1]
-- | ```
-- |
foreign import reverse :: forall a. Array a -> Array a
foreign import reverse :: Array ~> Array

-- | Flatten an array of arrays, creating a new array.
-- |
Expand Down
20 changes: 10 additions & 10 deletions src/Data/Array/NonEmpty.purs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,31 +162,31 @@ fromArray xs
| otherwise = Nothing

-- | INTERNAL
unsafeFromArray :: forall a. Array a -> NonEmptyArray a
unsafeFromArray :: Array ~> NonEmptyArray
unsafeFromArray = NonEmptyArray

unsafeFromArrayF :: forall f a. f (Array a) -> f (NonEmptyArray a)
unsafeFromArrayF = unsafeCoerce

fromNonEmpty :: forall a. NonEmpty Array a -> NonEmptyArray a
fromNonEmpty :: NonEmpty Array ~> NonEmptyArray
fromNonEmpty (x :| xs) = cons' x xs

toArray :: forall a. NonEmptyArray a -> Array a
toArray :: NonEmptyArray ~> Array
toArray (NonEmptyArray xs) = xs

toNonEmpty :: forall a. NonEmptyArray a -> NonEmpty Array a
toNonEmpty :: NonEmptyArray ~> NonEmpty Array
toNonEmpty = uncons >>> \{head: x, tail: xs} -> x :| xs

fromFoldable :: forall f a. Foldable f => f a -> Maybe (NonEmptyArray a)
fromFoldable = fromArray <<< A.fromFoldable

fromFoldable1 :: forall f a. Foldable1 f => f a -> NonEmptyArray a
fromFoldable1 :: forall f. Foldable1 f => f ~> NonEmptyArray
fromFoldable1 = unsafeFromArray <<< A.fromFoldable

toUnfoldable :: forall f a. Unfoldable f => NonEmptyArray a -> f a
toUnfoldable :: forall f. Unfoldable f => NonEmptyArray ~> f
toUnfoldable = adaptAny A.toUnfoldable

toUnfoldable1 :: forall f a. Unfoldable1 f => NonEmptyArray a -> f a
toUnfoldable1 :: forall f. Unfoldable1 f => NonEmptyArray ~> f
toUnfoldable1 xs = unfoldr1 f 0
where
len = length xs
Expand Down Expand Up @@ -244,10 +244,10 @@ head = adaptMaybe A.head
last :: forall a. NonEmptyArray a -> a
last = adaptMaybe A.last

tail :: forall a. NonEmptyArray a -> Array a
tail :: NonEmptyArray ~> Array
tail = adaptMaybe A.tail

init :: forall a. NonEmptyArray a -> Array a
init :: NonEmptyArray ~> Array
Comment on lines +247 to +250
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this is the most appropriate use, since the inner values are changing. Took inspiration from List library:
https://github.com/purescript/purescript-lists/blob/62900a56f6864638c952575dfd211a1cc55be7da/src/Data/List/NonEmpty.purs#L149-L153
By the same logic, we could also convert many others, such as head and tail of Array, as was done in this List PR:
purescript/purescript-lists#101
I wonder if this will add to confusion for beginners.

There's also some potentially misleading info in our definition of natural transformation.

A natural transformation is a mapping between type constructors of kind Type -> Type where the mapping operation has no ability to manipulate the inner values.

the Functor constraint is not enforced here; that the types are of kind Type -> Type is enough for our purposes.

So maybe that first paragraph should be changed to:

the mapping operation has no ability to manipulate the types of the inner values

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You’re reading the docs in a different way from what is intended there, because it’s not actually possible to write a function f ~> g which does manipulate the inner values in the way that is meant there, the compiler won’t let you.

In most cases, all that the “cannot manipulate the inner values” condition means is that every element of type a in the result also occurred in the input. So reverse, init, and even head are all perfectly valid natural transformations. An invalid one would be eg map (_ + 1), but of course that wouldn’t compile as you’d get something like “can’t match a0 with Int”.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this will add to confusion for beginners.

For what it is worth, the fact that head and reverse weren't defined via natural transformations confused this beginner. I was reading Jordan's reference and my first thought was "I bet reverse has a natural transformation in it's type signature". I wonder if we could use ~> in the code as an example of best practice (assuming that it is best practice) and put comments in such as "This type signature could be expressed as forall a. ...".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it is a best practice, it's not really very significant whether you use ~> or not. I just think we should avoid it in the core libraries because it's another thing to learn, it's harder to search for in Pursuit, and because it's awkward when you have cases which don't quite fit into natural transformations like tail :: forall a. List a -> Maybe (List a); if you wanted to write that with ~> you'd need to use a type like tail :: List ~> Compose Maybe List which I think we'd all probably agree is not appropriate.

init = adaptMaybe A.init

uncons :: forall a. NonEmptyArray a -> { head :: a, tail :: Array a }
Expand Down Expand Up @@ -309,7 +309,7 @@ alterAt i f = A.alterAt i f <<< toArray
intersperse :: forall a. a -> NonEmptyArray a -> NonEmptyArray a
intersperse x = unsafeAdapt $ A.intersperse x

reverse :: forall a. NonEmptyArray a -> NonEmptyArray a
reverse :: NonEmptyArray ~> NonEmptyArray
reverse = unsafeAdapt A.reverse

concat :: forall a. NonEmptyArray (NonEmptyArray a) -> NonEmptyArray a
Expand Down