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

Fix wxscheduler issues #602

Merged
merged 2 commits into from
Mar 7, 2022

Conversation

christiansandberg
Copy link
Contributor

I just saw that you are planning on deprecating these schedulers but since I already made the fixes I'm making a PR anyway.

The Timer class used for scheduling actions has a few drawbacks:

  • Timers can't be started from other threads, making operators like observe_on useless.
  • Timers are quite slow. When doing an immediate schedule it is much faster to use CallAfter instead.

@dbrattli
Copy link
Collaborator

Hi, thanks for the PR. We are discussing if we should deprecate them so things are not decided yet. Activity is always a good sign of not deprecating.

@coveralls
Copy link

coveralls commented Feb 20, 2022

Coverage Status

Coverage decreased (-0.04%) to 93.354% when pulling 25ba53f on christiansandberg:fixes-for-wx-scheduler into f35ee16 on ReactiveX:master.

Copy link
Collaborator

@dbrattli dbrattli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing. PS: You might want to also make a PR against the reactivex branch that will be the next version of RxPY (code name ReactiveX)

@dbrattli
Copy link
Collaborator

dbrattli commented Mar 5, 2022

@christiansandberg Could I ask you to please update your PR against the latest v4 code, and I will make sure the scheduler will be part of the release and not be deprecated.

@christiansandberg
Copy link
Contributor Author

Oh okay, I thought you would merge both to master eventually. I'll make a new PR.

@dbrattli
Copy link
Collaborator

dbrattli commented Mar 5, 2022

PS: See instructions here for setting up the development environment: https://github.com/ReactiveX/RxPY#development

@dbrattli dbrattli self-requested a review March 6, 2022 08:08
* Could not schedule actions from other threads
* Slow `schedule` method
@christiansandberg
Copy link
Contributor Author

Ok, I've rebased on top of master now.

Copy link
Collaborator

@dbrattli dbrattli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Thanks for fixing!

@dbrattli dbrattli merged commit 0e5a07b into ReactiveX:master Mar 7, 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

Successfully merging this pull request may close these issues.

None yet

3 participants