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

Jump to bottom

fix alt for ZipList #150

Merged
merged 2 commits into from
Dec 26, 2020
Merged

fix alt for ZipList #150

merged 2 commits into from
Dec 26, 2020

Conversation

xgrommx
Copy link
Contributor

@xgrommx xgrommx commented Jul 31, 2018

No description provided.

@garyb
Copy link
Member

garyb commented Jul 31, 2018

Seems reasonable to me. Anyone else?

natefaubion
natefaubion reviewed Jul 31, 2018
View reviewed changes
src/Data/List/ZipList.purs Outdated
@@ -46,7 +47,7 @@ instance applicativeZipList :: Applicative ZipList where
pure = ZipList <<< repeat

instance altZipList :: Alt ZipList where
alt = append
alt (ZipList xs) (ZipList ys) = ZipList $ (xs <> drop (length xs) ys)
Copy link
Contributor

Choose a reason for hiding this comment

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

Unnecessary $, or unnecessary parens.

@garyb garyb added the type: breaking change A change that requires a major version bump. label Nov 11, 2018
@hdgarrood
Copy link
Contributor

Can you describe what is being fixed here? Also, do you have a proof that this version satisfies all the relevant laws?

@hdgarrood
Copy link
Contributor

Also won’t this diverge if the first argument is infinite?

@hdgarrood
Copy link
Contributor

hdgarrood commented May 7, 2019 •
edited
Loading

Ah ok, so I presume this is motivated by the fact that the current version doesn’t satisfy the Alternative distributivity law

(f <|> g) <*> x == (f <*> x) <|> (g <*> x)

since with the current instance, the left hand side will be a list of length min(length f + length g, x), whereas the right hand side will be a list of length min(length f, length x) + min(length g, length x). For example, with f = g = ZipList ((_+1) : nil) and x = ZipList (0 : nil) we get the left hand side being ZipList (1 : nil) but the right hand side being ZipList (1 : 1 : nil).

@hdgarrood hdgarrood mentioned this pull request May 25, 2020
Closed
@kl0tl kl0tl mentioned this pull request Dec 19, 2020
Closed
57 tasks
@hdgarrood
Copy link
Contributor

I'm going to merge this momentarily, just as soon as I've written a satisfactory commit message

@hdgarrood hdgarrood merged commit 0550216 into purescript:master Dec 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@natefaubion natefaubion natefaubion left review comments

@LiamGoodacre LiamGoodacre LiamGoodacre approved these changes

@hdgarrood hdgarrood hdgarrood approved these changes

@thomashoneyman thomashoneyman thomashoneyman approved these changes

Assignees
No one assigned
Labels
type: breaking change A change that requires a major version bump.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@xgrommx @garyb @hdgarrood @natefaubion @LiamGoodacre @thomashoneyman