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

Add support for delegation and return values with @yieldfrom #57

Merged
merged 25 commits into from
Jun 4, 2023

Conversation

gerlero
Copy link
Member

@gerlero gerlero commented Nov 29, 2022

This is a basic implementation of delegation to a subgenerator inspired by PEP 380. Delegation with a plain @yieldfrom iter expression (i.e., no assignment on the left) should work for any iterable, while arg = @yieldfrom f requires that f is actually a @resumable function, as I didn't find a standard mechanism for iterators to "return" values. Note that in the latter case, arg will be assigned the return value of f: this also means that it now makes sense for a @resumable function to also return a value, so I've removed the warning for this case.

Closes #49

EDITS: @yield_from -> @yieldfrom (QuantumSavory#8), typo

@Krastanov
Copy link
Member

I believe Ben, the original author, might not have the bandwidth to work on ResumableFunctions these days. Thus, I forked the repository into a new Semicoroutines.jl https://github.com/QuantumSavory/Semicoroutines.jl in order to support 1.10 and future julia versions.

@gerlero , could you consider making this pull request against the fork?

As this library is an important part of another project of mine, I plan to provide support at least for the next few years. I am certainly interested in having help from other contributors (and maybe merging things back here if ResumableFunctions.jl is revided).

A similar fork is being prepared for SimJulia

@gerlero
Copy link
Member Author

gerlero commented Apr 30, 2023

Sure enough. Thanks! I'll open the same PR against Semicoroutines.jl

@BenLauwens
Copy link
Collaborator

Please resolve conflicts so that this can be merged.

@gerlero
Copy link
Member Author

gerlero commented May 5, 2023

@BenLauwens The same PR is currently open against Semicoroutines.jl (at QuantumSavory#8). If possible, I'd prefer to wait for that one to be merged there, to ensure both packages remain compatible.

@Krastanov
Copy link
Member

@gerlero , give me a few hours to fix the tests, so we have JET and Aqua available. That might also already fix most of the merge conflicts.

@gerlero gerlero changed the title Add support for delegation and return values with @yield_from Add support for delegation and return values with @yieldfrom May 9, 2023
@gerlero
Copy link
Member Author

gerlero commented May 9, 2023

@Krastanov Let me know when I can start working on the merge conflicts for this one.

@Krastanov
Copy link
Member

Please go ahead! Tests and CI are already updated.

@Krastanov
Copy link
Member

I guess we need to decide whether to drop support for Julia 1.2... The current LTS is 1.6, anything older is not really supported upstream either.

@hdavid16
Copy link
Member

hdavid16 commented May 9, 2023

I support dropping it. I would go with LTS and up.

@gerlero
Copy link
Member Author

gerlero commented May 10, 2023

I couldn't easily figure out why that error occurs in Julia 1.2, and don't believe any additional effort would be really worth it with that Julia version. I'd be fine with dropping support for anything older than 1.6 LTS.

@Krastanov
Copy link
Member

Let's wait until next week when we will have a more extended discussion with Ben and bring it up then. I am also in favor of dropping pre-1.6 versions.

@Krastanov
Copy link
Member

In the meantime could you go ahead and modify the project.toml (to increase to julia 1.6 and to bump the version number for the package) and add a quick description to README in the change log section?

@gerlero
Copy link
Member Author

gerlero commented May 10, 2023

In the meantime could you go ahead and modify the project.toml (to increase to julia 1.6 and to bump the version number for the package) and add a quick description to README in the change log section?

Done. I've bumped just the patch component (to v0.6.3), pending/following discussion at #63, but it's up to you and others if you think this should get a different version number (e.g. 0.7 or 1.0).

@Krastanov Krastanov merged commit 727ba4a into JuliaDynamics:master Jun 4, 2023
10 checks passed
@Krastanov
Copy link
Member

all the cleanup from the move is now finished, so I am merging this. Thanks for the contribution!!!

@gerlero
Copy link
Member Author

gerlero commented Jun 4, 2023

@Krastanov thank you!

gerlero added a commit that referenced this pull request Sep 5, 2023
Krastanov added a commit that referenced this pull request Sep 6, 2023
* Revert Base.iterate definition changes from #57

* jet and changelog and trigger benchmark CI

* add explicit allocation tests

---------

Co-authored-by: Stefan Krastanov <github.acc@krastanov.org>
Co-authored-by: Stefan Krastanov <stefan@krastanov.org>
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

Successfully merging this pull request may close these issues.

Delegating to subgenerator (PEP 380)
5 participants