ocaml-batteries-team / batteries-included

Batteries Included project
http://ocaml-batteries-team.github.com/batteries-included/hdoc2/
Other
518 stars 106 forks source link

Add to_seq and of_seq to all data structures #636

Open UnixJunkie opened 9 years ago

UnixJunkie commented 9 years ago

From caml-list:


---
that contributions
adding `to_seq`, `of_seq` conversion functions to Batteries data
structures are warmly welcome. (If you are wondering what's a
practical step to help improving the "base library" ecosystem...)

You may find out that it is not in general trivial to implement these
(for the reason explained in this blog post:
http://gallium.inria.fr/blog/generators-iterators-control-and-continuations/),
but luckily all the hard lifting has already been done to support
functions to and from the Enum type, and generalizing these to also
provide sequence providers or consumers should not be too hard.

---
UnixJunkie commented 9 years ago

By the way, what are the signatures of to_seq and of_seq ?

c-cube commented 9 years ago

type 'a sequence = ('a -> unit) -> unit

val to_seq : 'a t -> 'a sequence
val add_seq : 'a t -> 'a sequence -> 'a t (* often very useful, and makes [of_seq] trivial *)
val of_seq : 'a sequence -> 'a t

roughly, to_seq is the flipped version of iter, and of_seq folds on the sequence (using a ref for immutable types) to add the values.

Example for BatList:


type 'a sequence = ('a -> unit) -> unit

let to_seq l yield = iter yield l

let add_seq l seq =
  let l = ref (List.rev l) in
  seq (fun x -> l := x :: !l);
  List.rev !l

let of_seq seq = add_seq [] seq
c-cube commented 9 years ago

note that the BatList implementation can be made more efficient using the mutability tricks already present for tailrec map, etc. Here it is just a small example.

UnixJunkie commented 9 years ago

thanks for all the details !

UnixJunkie commented 6 years ago

maybe we should import the recent Seq module from the stdlib https://caml.inria.fr/pub/docs/manual-ocaml/libref/Seq.html then add of_seq, to_seqi and to_seq to all modules

thizanne commented 6 years ago

@fkhina did some work on this. I plan to review it more thoroughly and propose the PR when I'm done with my thesis (I expect in a few weeks).

Le mar. 11 sept. 2018 à 09:12, Francois Berenger notifications@github.com a écrit :

maybe we should import the recent Seq module from the stdlib https://caml.inria.fr/pub/docs/manual-ocaml/libref/Seq.html then add of_seq, to_seqi and to_seq to all modules

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ocaml-batteries-team/batteries-included/issues/636#issuecomment-420171452, or mute the thread https://github.com/notifications/unsubscribe-auth/ABo8UrHCfFAz7227YfpHImDaose_pM5Sks5uZ2JRgaJpZM4F05R5 .

gasche commented 6 years ago

That would be very nice. Note that we can reuse code from the standard library (no licensing issues etc.), but I would prefer if we respected our usual standards of having a test for each function.

In #876 I proposed to make a 2.9.0 release for 4.07.0 but without Seq support (I don't have the time to work on it right now). If you have opinions on this, please go over there and voice it.

UnixJunkie commented 6 years ago

Don't hesitate to send the PR. We can edit it before accepting it so, don't wait for you to have time. I may have time in some days.

UnixJunkie commented 2 years ago

@thizanne @fkhina do you still have some code that needs to be contributed on that?

thizanne commented 2 years ago

I do still have the student-project code of @kahinaFekir, but as far as I remember it needed some last bits of polishing before a submission. Unfortunately I'm not sure when I'll have the time to do that (most likely unit is months, and "just pushing the raw stuff" wouldn't help). @kahinaFekir is of course welcome to propose the patch for inclusion herself, otherwise I'll try to get this to an end at some point.

thizanne commented 2 years ago

After reading again, it's worth a bit of precision: the code itself is good quality, and I remember Kahina getting deserved flying marks for it. However, I also kind of remember that some stuff had been deliberately considered as out of topic and/or future work for the finitely-timed project matters, and this is the part that I would need to assess before blindly submitting a PR.

Le ven. 22 oct. 2021 à 22:30, Thibault Suzanne @.***> a écrit :

I do still have the student-project code of @kahinaFekir https://github.com/kahinaFekir, but as far as I remember it needed some last bits of polishing before a submission. Unfortunately I'm not sure when I'll have the time to do that (most likely unit is months, and "just pushing the raw stuff" wouldn't help). @kahinaFekir https://github.com/kahinaFekir is of course welcome to propose the patch for inclusion herself, otherwise I'll try to get this to an end at some point.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ocaml-batteries-team/batteries-included/issues/636#issuecomment-949941945, or unsubscribe https://github.com/notifications/unsubscribe-auth/AANDYUWISDWLJIKJPU4H4XLUIHCW3ANCNFSM4BOTSR4Q . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

UnixJunkie commented 2 years ago

@thizanne Maybe send a raw PR. I'll try to review it and merge what's mergeable.