search

r-lib / coro

Coroutines for R
https://coro.r-lib.org/
Other
160 stars 8 forks source link

Add support for python objects #37

Closed t-kalinowski closed 2 years ago

t-kalinowski commented 2 years ago

This PR adds an as_iterator() method for a base python object. This enables usage like this with TensorFlow Datasets:

ds <- ...

loop(for(batch in ds) {
  ...
})

Without this, users need to instead explicitly call reticulate::as_iterator(ds), before passing to coro::loop(), which can be confusing because of the name clash between coro::as_iterator(), and also not necessary for most other iterables that are automatically converted to an iterator.

t-kalinowski commented 2 years ago

I think we don't even need a separate method for python.builtin.iterator, the method for python.builtin.object covers all cases. I'm going to remove it.

lionel- commented 2 years ago

Now this is just missing a NEWS bullet :)

t-kalinowski commented 2 years ago

Done!

lionel- commented 2 years ago

Thanks! Do you need that on CRAN? If yes, I'll probably release right away, otherwise this might sit on github for months. Do you have any other planned changes for coro?

t-kalinowski commented 2 years ago

This came out of working on updates to the Tensorflow website, which is still ongoing work. I don’t have any other coro enhancements planned presently, but other things might still shake out in the next two weeks while we’re updating the TF examples. @dfalbel what do you think?

t-kalinowski commented 2 years ago

@lionel- If it's ok with you, let's wait ~ 2 weeks. I'll ping you when we need this on CRAN.

lionel- commented 2 years ago

That sounds good.

t-kalinowski commented 2 years ago

Hi @lionel-, the dev version is working great for us and I don't think we'll have any additional PRs to coro in the near term. When you get a chance, can you please publish to CRAN?

lionel- commented 2 years ago

Great, I'll do that!