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

define pairs for Cons #580

Open
jlapeyre opened this issue Feb 14, 2020 · 10 comments
Open

define pairs for Cons #580

jlapeyre opened this issue Feb 14, 2020 · 10 comments

Comments

@jlapeyre
Copy link

jlapeyre commented Feb 14, 2020

With this definition

Base.pairs(x::Cons) = (first(p) => last(p) for p in enumerate(x))

findall, findfirst, etc. will work with linked lists.

Or maybe even, since there seems to be no default method for pairs,

Base.pairs(x) = (first(p) => last(p) for p in enumerate(x))

(Of course, the latter would go in Julia, not a package)

@jlapeyre
Copy link
Author

And this allows findlast to work with Cons

Base.Iterators.Reverse(x::Cons) = reverse(x)

@oxinabox
Copy link
Member

oxinabox commented Feb 14, 2020

Oh thats pretty cunning.
Does Base.pairs(x::Cons) = enumerate(x) work?
(some things treat Pair and two-tuple the same)

@jlapeyre
Copy link
Author

Does Base.pairs(x::Cons) = enumerate(x) work?

Yes. Good eye. I wonder then if Base.pairs(x) = enumerate(x) is a good idea. This would assume that any type that has not defined a method for pairs, and that can be iterated over, can sensibly be indexed by one-dimensional LinearIndices. On the other hand, new types could come to rely on this without realizing it, which would make it hard to take back later if it turns out to be a bad idea. At a minimum, defining the method for x::Cons makes sense.

@jlapeyre
Copy link
Author

I wonder if Cons is actually used by anyone, or if this is all just for fun. This came up because I was thinking of a demonstration of the advantages of generic functions in Julia compared to Common Lisp. Since singly-linked lists are so common in in CL, I wanted to show that findall compiles to efficient code for many data structures, including lists. But, it didn't work. Of course, demonstrating how easy it is to fix is impressive as well.

@oxinabox
Copy link
Member

we have a few other packages for this kind of list.
FunctionalCollections.jl, and Lists.jl

It might be a good idea to remove this from DataStructures and leaver it to a package that is more into that kind of collection;
to thus decrease our code surface to get ready to release 1.0

@karan0299
Copy link
Contributor

karan0299 commented Feb 21, 2020

So, in the above discussion should I infer that Cons DataStructure be removed from this package?
If so, I would like to work on this.
Otherwise also, I would like to take up this issue.
Thanks!

@oxinabox
Copy link
Member

No decision has been made here. Yet.
Options:

  1. Remove Cons from this package (potentially putting it elsewhere)
  2. Add pairs definition to this package
  3. Add fallback pairs definition to Base

@karan0299
Copy link
Contributor

I think 2nd option would be best in this scenario
For 3rd option I think, I have to change the Julia/JuliaLang repo.
For now, I cannot figure out where to put Cons if we remove from this package.

@jlapeyre
Copy link
Author

It might be worth mentioning this in a Julia language issue.

@oxinabox
Copy link
Member

@jlapeyre feel encouraged to open such an issue and link to here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants