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

Subscribe signature(s) #430

Closed
erikkemperman opened this issue Jun 12, 2019 · 5 comments
Closed

Subscribe signature(s) #430

erikkemperman opened this issue Jun 12, 2019 · 5 comments

Comments

@erikkemperman
Copy link
Collaborator

erikkemperman commented Jun 12, 2019

I'm confused about Observable.subscribe and Observable.subscribe_. If we have the latter, why should the former have to be polymorphic? The reason I ask is I'm trying to reduce the typehint warnings, and mypy is unhappy about the difference between Subject.subscribe and the signature in typing.Observable...

If I can get things to work, would it make sense to simply have subscribe accept a single argument (type Observer) and let subscribe_ handle the case of varying number of callbacks? And actually, the signature in typing.Observable.subscribe also seems a bit off, it says the Observer argument is optional, but does it make sense to invoke this method without an observer?

I'm probably missing some history here, sorry if this is a silly question...

@jcafhe
Copy link
Collaborator

jcafhe commented Jun 12, 2019

This is the rx v1 behavior, i.e. the 2 following examples would work:
source.subscribe(handler_on_next, handler_on_error, handler_on_complete)
source.subscribe(observer)

This was discussed from this comment, when callbacks were only allowed in subscribe_, and observer in subscribe.
Personally, I prefer subscribe with callbacks (I've never used an Observer as a subscriber). Also I believe @dbrattli attempted to move the code to subscribe with callbacks but that would require updating a lot of operators. However, I guess that could be done and would be happy to contribute.

@erikkemperman
Copy link
Collaborator Author

Ah thanks, I had a feeling this came before but I could not find it!

So my takeaway from re-reading that thread, the consensus seems to be that subscribe should take callbacks, and we can have something like subscribe_observer to get rid of polymorphism.

Other than the names of the functions, which hopefully my IDE will help me with, I think I’m pretty close to making that work.

@dbrattli
Copy link
Collaborator

It makes sense to call subscribe without observer or callbacks. It's often used when you have side-effects in the chain e.g flat_map or tap and you just want to start/run things. Note that the name of subscribe in the continuation monad is runCont.

Yes, we would need something like subscribe_observer to help the operators. The crux is what to do with abc.Observable. I would like to avoid 2 subscribe methods. If we only have callbacks then that is ok with me but then the whole purpose of observers goes away (should we eventually remove them?)

@mysticfall
Copy link

I've started using RxPy recently, and I have a question about the signature of subscribe API.

In other languages with which I've used Rx API, they usually provide a typed variant of subscribe which accepts on_next/on_complete/on_error callbacks.

So, I was surprised to find that the equivalent in RxPy only provides an untyped version, even though Observable actually extends the generic base class Observable[T].

Is there a reason why we can't add a type hint to the more convenient version of subscribe API?

@dbrattli
Copy link
Collaborator

dbrattli commented Mar 5, 2022

This is fixed in the latest --pre release and will be fixed in the latest major release. There's only one subscribe now taking 3 callbacks (or 1 observer).

@dbrattli dbrattli closed this as completed Mar 5, 2022
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

4 participants