MNT/FIX: macosx change Timer to NSTimer instance #26022
Conversation
src/_macosx.m
Outdated
| self->timer = NULL; | ||
| return (PyObject*) self; | ||
| } | ||
|
|
||
| static PyObject* | ||
| Timer_repr(Timer* self) | ||
| { | ||
| return PyUnicode_FromFormat("Timer object %p wrapping CFRunLoopTimerRef %p", | ||
| return PyUnicode_FromFormat("Timer object %p wrapping NSTimerRef %p", |
| @@ -44,3 +45,26 @@ def new_choose_save_file(title, directory, filename): | |||
| # Check the savefig.directory rcParam got updated because | |||
| # we added a subdirectory "test" | |||
| assert mpl.rcParams["savefig.directory"] == f"{tmp_path}/test" | |||
|
|
|||
|
|
|||
| @pytest.mark.backend('macosx') | |||
There was a problem hiding this comment.
Perhaps the same test makes sense for other frameworks too?
There was a problem hiding this comment.
Sure, I just added it to test_backends_interactive instead. We'll see if all the other backends are also implemented properly ;)
|
Well, what do you know Gtk doesn't like 0 for the interval and macosx didn't work with it before either, so I just pushed up a new commit setting a minimum of 1 millisecond for the interval of timers. This seems reasonable to me and I can't imagine anyone is counting on timers < 1/1000 of a second and they are entirely unreliable at that speed as well because we don't allow fractional numbers in the first place. Edit: Seems like GtkCairo is the common failure point. We might need to increase the pause time and/or timer length to satisfy that backend. |
de03aea to
8866eff
Compare
|
To be clear, I don't think you need to fix the other backends right now; xfailing them and opening an issue to track that is fine too. |
It does work locally for me on gtk3cario, so I think it must have something to do with the remote slowness which I just updated hoping it would work. If you don't want the minimum 1-millisecond commit then we would have to xfail. However, luck would have it that either a new runner or QT release must have been put out within the last hour when I thought I had a fix up :) that is causing many more unrelated failures. and |
8b2bbac to
127d8ed
Compare
Newer macos frameworks now support blocks, so we can take advantage of that and inline our method call while creating and scheduling the timer all at once. This removes some of the reference counting and alloc/dealloc record keeping. In the mac framework, an interval of 0 will only fire once, so if we aren't a singleshot timer we need to ensure that the interval is greater than 0.
Some backends don't handle 0 interval timers, so we can just set a minimum of 1 millisecond for the resolution of the timer to ensure a positive interval passed to all backend timers.
127d8ed to
dc38128
Compare
PR summary
Newer macos frameworks now support blocks, so we can take advantage of that and inline our method call while creating and scheduling the timer all at once. This removes some of the reference counting and alloc/dealloc record keeping.
In the mac framework, an interval of 0 will only fire once, so we need to account for that code path as well, which is easier with the
repeat:option now. (See the first test block, which failed wheninterval=0on the timer but we wanted it to keep firing)PR checklist