Polish docstrings for static operators

[erikkemperman]

I notice that the generate_with_relative_time and generate static ops don't handle a scheduler parameter, but the docstring says:

RxPY/rx/__init__.py

Lines 491 to 494 in dc765aa

	def generate(initial_state: Any, condition: Predicate, iterate: Mapper) -> Observable:
	"""Generates an observable sequence by running a state-driven loop
	producing the sequence's elements, using the specified scheduler to
	send out observer messages.

For the moment I've removed the docstring bit about "using the specified" scheduler. But I wonder if an optional scheduler argument should be added here?

The newly added marble diagrams for case and for_in looked a bit "cramped" to my eyes. So I made them slightly wider, but if I missing some particular reason behind this then please let me know (@MainRo)!

Another question, all references to Observable in rx/__init__.py are actually to rx.core.observable.Observable -- probably just because it is already available as that class is being re-exported rx.Observable.

But from an API perspective, should the input Observables not actually be typing.Observable, so as not to tie the users to our particular concrete base class?

[coveralls]

Coverage increased (+0.3%) to 92.887% when pulling 022a4fc on erikkemperman:polish-static-docs into dc765aa on ReactiveX:master.

[jcafhe]

I think the resulting observable should be shifted to the right (+ 3 hyphens), i.e. the 'a' observable is subscribed when the source observable emits 1.

[erikkemperman]

        ---1---|
        a----1----2----3----4---|
         b----10---20---30---|
        [case(mapper, { 1: a, 2: b })]
        --------1----2----3----4---|

Do you mean like this? I don't know, I tried to keep the vertical alignment the same... I think the point is to emphasize that the result sequence is the same as a. But I got this wrong before :-)

[jcafhe]

Should subscribe be qualified as Optional with None as a default value in the signature? Don't know if it's worth to call create with no subscribe function thougth.

[erikkemperman]

Ah, good catch. Sloppy/paste, I guess... I'll amend.

[jcafhe]

Not sure here, but I think there should be 6 hyphens instead of 4 between 3 and 11, i.e. 2 from the end of 'a' observable + 4 from the start of the 'b' observable.

[erikkemperman]

        a----1----2--|
         b----10---20--|
        [for_in((a, b), lambda i: i+1)]
        -----2----3------11----21--|

Like this?

[jcafhe]

Exactly 👍

[jcafhe]

Trailing period. Sorry to bother you

[erikkemperman]

No bother, thanks!

[jcafhe]

Good job! 👍

For the moment I've removed the docstring bit about "using the specified" scheduler. But I wonder if an optional scheduler argument should be added here?

Yes, that could be great, just by adding the common 'pattern'
_scheduler = scheduler or scheduler_ or default_scheduler

But from an API perspective, should the input Observables not actually be typing.Observable, so as not to tie the users to our particular concrete base class?

I agree too.

[erikkemperman]

OK, I've removed the commit about the marble diagrams for now -- I guess I would prefer if @MainRo could take a look, if there's a bit of time, at making them slightly more "balanced" (rather than me risking inadvertently changing something in his effort). It's a really minor point, obviously.

And I think I've addressed the other remarks, about trailing period and incorrect Optional.

I'll see if I can quickly propose something about using typing.Observable where appropriate, as well as adding scheduler arguments where it might make sense.

[dbrattli]

👍

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.