Oberservable.interval does not work with the AsyncIO Scheduler for Float Periods

[frederikaalund]

The following program illustrates the problem:

import rx
import asyncio

# Bug is with the AsyncIO scheduler
scheduler = rx.concurrency.AsyncIOScheduler()
# Works with the default scheduler (timeout)
#scheduler = None

subscription = rx.Observable.interval(1000.0, scheduler=scheduler) \
        .subscribe(print, print, print)

try:
        loop = asyncio.get_event_loop()
        loop.run_forever()
except:
        pass

subscription.dispose()

No output is generated with the AsyncIO scheduler. Replace 1000.0 with 1000 and it works. Both cases work with the default scheduler (timeout).

Very subtle bug. I couldn't figure out why no events were firing until I isolated my code to the above.

[dbrattli]

Master or develop branch? I think this should already be fixed in the develop branch? I should probably create a new release from develop this weekend.

[frederikaalund]

Ah, sorry forgot to specify that. I tested with the master branch. Sounds good if it's already fixed! Looking forward to the new release.

Feel free to close this issue if it's fixed in develop (I don't have time to test atm).

[dbrattli]

The question is really if we should use seconds for all calls where we specify time since that is what's used for Python standard libraries? Rx.NET uses only time objects, while RxJS uses milliseconds.

[frederikaalund]

That is a good point and an unfortunate inconsistency between the Python standard and existing Rx implementations. RxJava implements a third variant which takes the time unit as a parameter.

In the hope that Rx will eventually get standardised, my go-to preference is to follow the example of one of the existing implementations. However, consistency with the target language (and its standards) trumps that argument. This is the same reason I think it is a good choice to use snake_case instead of camelCase. I don't like renaming for the sake of renaming (even if the new name suits the function better) but I have nothing against simple mappings.

So in this case I think that using seconds is the sane choice. As you said yourself, seconds are used everywhere in the Python standard libraries. Using seconds in RxPY as well makes it easier to use for Python users that are new to Rx. My best guess is that a similar argument was used for RxJS (since milliseconds are used everywhere in the Javascript standard libraries).

However, I do believe this is a "RxPY 2.0" kind of change.

Cheers for fixing the issue btw.

[lock[bot]]

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.