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.

Sign up for GitHub

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

Jump to bottom

Code removal #3992

Merged
merged 13 commits into from Feb 6, 2015
Merged

Code removal #3992

merged 13 commits into from Feb 6, 2015

Conversation

tacaswell
Copy link
Member

For some reason I thought this would be 'quick'. There are still a few more deprecated things that need to be removed, but this is a good start.

We need to make a decision about the future of finance.py.

I did my best to make sure the commits are atomic so any of them can be dropped now (or reverted later) if need be.

@tacaswell tacaswell added this to the v1.5.x milestone Jan 11, 2015
@WeatherGod
Copy link
Member

I have changed my mind on removal of qt4_compat.py. I encountered a
conundrum while writing my book. I had to choose between demoing with
qt4_compat.py which has been in mpl since v1.1, but deprecated in v1.4, or
demoing with qt_compat.py, which was introduced in v1.4, so it required the
latest version of matplotlib. I don't think a single release cycle would be
sufficient.

On Sat, Jan 10, 2015 at 6:45 PM, Thomas A Caswell notifications@github.com
wrote:

For some reason I thought this would be 'quick'. There are still a few
more deprecated things that need to be removed, but this is a good start.

We need to make a decision about the future of finance.py.

I did my best to make sure the commits are atomic so any of them can be

dropped now (or reverted later) if need be.

You can merge this Pull Request by running

git pull https://github.com/tacaswell/matplotlib code_removal

Or view, comment on, or merge it at:

#3992
Commit Summary

  • MNT : removed Axis.set_scale
  • MNT : removed deprecated code in finance.py
  • MNT : remove deprecated delaunay code
  • MNT : remove deprecated code from Annotation
  • MNT : remove qt4_compat.py
  • MNT : removed sphinxext/ipython_* files
  • MNT : remove code to handle callable legend handlers
  • MNT : remove LineCollection.color
  • MNT : remove deprecated value of kwarg in tri.tripcolor
  • MNT : remove set_colorbar from ScalarMappable
  • MNT : removed deprecated method/kwargs from patheffects
  • MNT : remove testing.image_util.py
  • MNT : remome mlab.FIFOBuffer
  • MNT : remove mlab.premca
  • MNT : Remove NavigationToolbar2QTAgg

File Changes

Patch Links:

—
Reply to this email directly or view it on GitHub
#3992.

@tacaswell
Copy link
Member Author

@WeatherGod removed the removal

ianthomas23
ianthomas23 reviewed Jan 12, 2015
View reviewed changes
lib/matplotlib/tri/tripcolor.py
@@ -65,6 +64,10 @@ def tripcolor(ax, *args, **kwargs):
shading = kwargs.pop('shading', 'flat')
facecolors = kwargs.pop('facecolors', None)

if shading not in ['flat', 'gouraud']:
raise ValueError("shadding must be one of ['flat', 'gouraud'] "
Copy link
Member

Choose a reason for hiding this comment

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

Typo: 'shadding' -> 'shading'

@ianthomas23
Copy link
Member

I'm happy with the removal of the deprecated kwarg in tripcolor.

For the delaunay module I had intended keeping it around for a couple of release cycles, but I don't have a particularly strong feeling either way. When it does finally go we will receive some complaints, but I don't know that we'll receive more complaints if we remove it sooner rather than later.

@tacaswell
Copy link
Member Author

@ianthomas23 I suspect the amount of complaining will be independent of when we do it. I will defer to you on when it goes.

@tacaswell
Copy link
Member Author

@WeatherGod On consideration, your argument is essentially that all re-factoring needs more than one release cycle.

@WeatherGod
Copy link
Member

It is a bit more nuanced than that. qt4_compat.py and qt_compat.py are
intended to act as compatibility layers. Their entire purpose is to be a
stable platform for developers to avoid the weirdness between different qt
bindings. qt4_compat allows developers to target version 4 of the qt
bindings, while qt_compat allows developers to target version 5+ of the qt
bindings, regardless of which bindings are installed. The compatibility
layers are intended to help developers reach as wide of an audience as
possible. By deprecating qt4 over a single release cycle, you are asking
developers to choose to use a deprecated, but widely used qt4_compat, or
the newly created, not yet widely used qt_compat.py, completely defeating
the purpose of a compatibility layer.

As an example of a refactoring that I have zero problems with taking only a
single release cycle to do: the deprecation of setting the backend via
command-line in favor of an environment variable. I highly doubt that
feature was widely used, nor was it a decent way of doing things in the
first place. It is also fairly trivial to change any scripts to simply not
provide the information via command-line, and instead provide it via an
exported variable. Switching between qt4 and qt5+ is probably not nearly as
trivial.

On Mon, Jan 19, 2015 at 4:04 PM, Thomas A Caswell notifications@github.com
wrote:

@WeatherGod https://github.com/WeatherGod On consideration, your
argument is essentially that all re-factoring needs more than one
release cycle.

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

@ianthomas23
Copy link
Member

@tacaswell: OK, I think we should keep the delaunay code for one more release cycle before removing it.

@tacaswell
MNT : removed Axis.set_scale
48932c1 
Deprecated in PR matplotlib#1917 and commit 4003603
@tacaswell
MNT : removed deprecated code in finance.py
48b8e6b 
Deprecated in matplotlib#1920

merged to master in 17216bb
@tacaswell
MNT : remove deprecated code from Annotation
310cab1 
Deprecated in PR matplotlib#2351

Merged to master as da6c6b5
@tacaswell
MNT : removed sphinxext/ipython_* files
843e16f 
Deprecated in matplotlib#2671 / 71c77f0

ipython is now a doc-build dependency
@tacaswell
MNT : remove code to handle callable legend handlers
d17863f 
Deprecated in matplotlib#2442 / 9e0be95
@tacaswell
MNT : remove LineCollection.color
f431d48 
Deprecated in c17e217 (2005-02-01)
@tacaswell
MNT : remove deprecated value of kwarg in tri.tripcolor
a78746d 
Deprecated in matplotlib#850 / 360887a

Also added validation on value of shading to only be in the supported
set.

attn @ianthomas23
@tacaswell
MNT : remove set_colorbar from ScalarMappable
78aabc3 
Deprecated in matplotlib#2055 / 148ed82
@tacaswell
MNT : removed deprecated method/kwargs from patheffects
f98c484 
Deprecated in matplotlib#2462 / 84e0063

attn @pelson had to known-fail a test which was using the
proxy renderer to verify that PathEffectRender was working
correctly.
@tacaswell
MNT : remove testing.image_util.py
e0fd416 
Deprecated matplotlib#2714 / 400754f
@tacaswell
MNT : remome mlab.FIFOBuffer
2f30efc 
Deprecated in 7634812
@tacaswell
MNT : remove mlab.premca
1f9023d 
Deprecated in dc13d25 (2009-11-03)
@tacaswell
MNT : Remove NavigationToolbar2QTAgg
056ba7a 
Deprecated is matplotlib#2680 / e66039b
@efiring
Copy link
Member

efiring commented Feb 6, 2015

@tacaswell, Is there a clean way to move this forward? It looks like most of the removals are non-controversial. Maybe the thing to do is to move each of the problematic changesets into its own PR, and then merge the remaining majority in the present PR.

@tacaswell
Copy link
Member Author

I already have removed the changes that had protests. I have them as commits on my computer, but haven't gotten around to cherry-picking them on top of current master.

efiring added a commit that referenced this pull request Feb 6, 2015
@efiring
Merge pull request #3992 from tacaswell/code_removal
e2a8342 
Code removal
@efiring efiring merged commit e2a8342 into matplotlib:master Feb 6, 2015
@tacaswell tacaswell deleted the code_removal branch February 7, 2015 02:58
@QuLogic
Copy link
Member

QuLogic commented Dec 24, 2015

Here, the sphinxext/ipython_* files were removed, but doc/conf.py still has a fallback from IPython's to matplotlib's ipython_console_highlighting extension. Since that no longer exists, should that fallback also be removed?

@jenshnielsen
Copy link
Member

@QuLogic Sounds right, will you do a PR?

@QuLogic
Copy link
Member

QuLogic commented Dec 24, 2015

I'll put something together as part of #5371.

@mdboom mdboom mentioned this pull request Dec 29, 2015
Merged
@mdboom
Copy link
Member

mdboom commented Dec 29, 2015

I'll put something together as part of #5371.

It looks like this was forgotten before #5371 was merged. No worries about that, I made #5762...

@QuLogic
Copy link
Member

QuLogic commented Dec 30, 2015

Ahh, oops, I meant to say #5743!

@QuLogic QuLogic modified the milestones: v1.5.0, proposed next point release (2.1) Dec 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v1.5.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@tacaswell @WeatherGod @ianthomas23 @efiring @QuLogic @jenshnielsen @mdboom @afvincent