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

Improve the itertools API (RFC 49) #2190

Merged
merged 1 commit into from
Aug 30, 2017
Merged

Improve the itertools API (RFC 49) #2190

merged 1 commit into from
Aug 30, 2017

Conversation

Theodus
Copy link
Contributor

@Theodus Theodus commented Aug 23, 2017

This PR makes the changes described in RFC 49. These include making
fold non-partial and adding the fold_partial, filter_map, and
flat_map methods. All classes in the itertools package other than
Iter have been removed.

Resolves #2189

This PR makes the changes described in RFC 49. These include making
`fold` non-partial and adding the `fold_partial`, `filter_map`, and
`flat_map` methods. All classes in the itertools package other than
`Iter` have been removed.

Resolves #2189
@Theodus Theodus added the changelog - changed Automatically add "Changed" CHANGELOG entry on merge label Aug 23, 2017
@jemc jemc mentioned this pull request Aug 24, 2017
@SeanTAllen
Copy link
Member

SeanTAllen commented Aug 25, 2017

@Theodus before this gets merged can you write up the release notes for this for when it goes out and put them on here.

You would need to explain the change, why it was done and include code that shows how to replace previous code usage with new API. Its probably going to be somewhat long based on the number of changes this makes. I don't want to have this merged without documentation that will go in the release notes for how to end users can update their code. I'm marking this DO NOT MERGE until we have how to upgrade on this PR so it can be included in next release and so in the meantime, there is something for folks tracking master to refer to.

@SeanTAllen SeanTAllen added the do not merge This PR should not be merged at this time label Aug 25, 2017
@Theodus
Copy link
Contributor Author

Theodus commented Aug 25, 2017

Improve the itertools API

The implementation of RFC #49 makes many changes to the itertools package. The first major change is that all classes other than Iter have been removed.

This means that Cycle, MapFn, Filter, Repeat, Take, etc. must be replaced by the equivalent Iter functions of chain, map, filter, repeat_value, take, etc. For example:

Zip2[I64, String](
  [as I64: 1; 2; 3].values(),
  ["a"; "b"; "c"].values())

Should be replaced with

Iter[I64]([as I64: 1; 2; 3].values())
  .zip[String](["a"; "b"; "c"].values())

Another big change is that fold has become non-partial and has had its arguments switched so that the lambda is placed last. For example:

let sum =
  try  
    Iter[I64]([1; 2; 3].values())
      .fold[I64]({(sum: I64, n: I64): I64 => sum + n }, 0)?
  else
    I64(0)
  end

Should be replaced by

let sum =
  Iter[I64]([1; 2; 3].values())
    .fold[I64](0, {(sum: I64, n: I64): I64 => sum + n })

If the lambda argument to fold needs to be partial, then you should use the new fold_partial method.

Other methods have been added to the Iter class such as flat_map, filter_map, as well as methods that allow lambdas with ref receivers for stateful transformations. These methods have a "_stateful" suffix, such as map_stateful and filter_stateful.

@SeanTAllen SeanTAllen removed the do not merge This PR should not be merged at this time label Aug 25, 2017
@SeanTAllen
Copy link
Member

Thanks @Theodus. I've created a release-0.19.0 branch in the website repo and started the release notes for 0.19.0 with your itertools changes.

@SeanTAllen SeanTAllen merged commit a4fbcd8 into ponylang:master Aug 30, 2017
ponylang-main added a commit that referenced this pull request Aug 30, 2017
@Theodus Theodus deleted the itertools branch August 30, 2017 19:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog - changed Automatically add "Changed" CHANGELOG entry on merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants