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

MNT: Deprecate idle_event and remove it from all but wx backends #4544

Merged
merged 2 commits into from Jul 3, 2015
Merged

MNT: Deprecate idle_event and remove it from all but wx backends #4544

merged 2 commits into from Jul 3, 2015

Conversation

ahaldane
Copy link
Contributor

This PR deprecates idle_event and removes code related to it from the gtk backend.

The reasoning for this is discussed in #4534, but briefly, idle_event functionality is broken or missing in all but the wx backends (and doesn't work very well there either) and code related to it causes spurious warning in the gtk backend (see #3769).

The animations module should be used instead of idle_event for animations.

Please let me know if there is anything else to add.

@@ -2331,6 +2331,10 @@ def on_press(event):
cid = canvas.mpl_connect('button_press_event', on_press)

"""
if s == 'idle_event':
warnings.warn("idle_event is only implemented for the wx backend, "
"and will be removed in matplotlib 2.0. Use the "
Copy link
Member

Choose a reason for hiding this comment

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

Can you bump this to 2.1? I want to hold the line that the only API changes in 2.0 will be style changes.

Copy link
Member

Choose a reason for hiding this comment

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

Should this not use a MatplotlibDeprecationWarning from https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/cbook.py#L37 ?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, to what @jenshnielsen is asking.

@tacaswell
Copy link
Member

For reference it seems this has been non-functional in tk + qt since 83b71a8 (JDH in June 2008).

There may be other uses for idle event, for example an alternate way to deal with the auto-redrawing of stale figures is to stick a plt.draw_all() on to the idle event call back.

On the other hand, this has never been functional and no one has complained so there probably is not much demand for it.

Some code also needs to be removed from the qt4/qt5 backends.

This needs an entry in doc/dev/api_changes.

@tacaswell tacaswell added this to the next point release milestone Jun 21, 2015
This commit removes code related to `idle_event` in the gtk backend,
where it caused spurious warnings, and adds a deprecation warning
on any use of idle_event. Idle event still works in wx for now.

Fixes #4534.
@ahaldane
Copy link
Contributor Author

Update: Added an entry to doc/dev/api_changes, removed some code from qt4/qt5 backends, and updated deprecation warning to 2.1.

@jenshnielsen and @tacaswell, I'm not sure I understood your comments about MatplotlibDeprecationWarning. That's the same as mplDeprecation I have currently, isn't it?

The code removal from qt5/qt4 was very minimal, but I don't see anythign else related to idle_event. (There is code related to draw_idle, but that is different than idle_event).

@jenshnielsen
Copy link
Member

I would normally use it as here https://github.com/matplotlib/matplotlib/blob/master/lib/matplotlib/delaunay/triangulate.py#L14
but I guess this is fine too

@tacaswell
Copy link
Member

The reason to use the version of the deprecation warning in cbook rather than the built-in warnings is that by default the built-in warnings are suppressed, where as the warnings in cbook are by default visible.

@tacaswell
Copy link
Member

@ahaldane Can you not rebase/squash every time? It makes it hard to keep track of and sometimes the path we took to get from point A to point B is useful for forensics later.

@ahaldane
Copy link
Contributor Author

updated to use warn_deprecated.

Sorry for squashing before, it's just a habit from other repositories.

@tacaswell
Copy link
Member

Thanks, no worries about the habits from other projects. I will let this sit a bit longer and plan to merge this tonight.

tacaswell added a commit that referenced this pull request Jul 3, 2015
@tacaswell tacaswell merged commit aad1619 into matplotlib:master Jul 3, 2015
@OceanWolf
Copy link
Contributor

We should probably remove the example(s) using idle_event as well then...

@tacaswell
Copy link
Member

@OceanWolf The only example I can find that uses idle_event is marked as deprecated in this PR.

@OceanWolf
Copy link
Contributor

Ahh, sorry, I missed it because I expected to see the entire example deleted. I don't understand why we need to have an example about a deprecated feature...

@tacaswell
Copy link
Member

A lot of maintaining a big project is not moving too fast and making sure
people have time to notice and adapt. Just marking the example as
deprecated is more conservative that deleting it now (we can always delete
it later).

Like doctors, first do no harm.

On Sat, Jul 4, 2015 at 10:20 AM OceanWolf notifications@github.com wrote:

Ahh, sorry, I missed it because I expected to see the entire example
deleted. I don't understand why we need to have an example about a
deprecated feature...


Reply to this email directly or view it on GitHub
#4544 (comment)
.

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

4 participants