NotificationsYou must be signed in to change notification settings
Fork7.9k
Star21.4k

MAINT finance module is deprecated#7295

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

QuLogic merged 2 commits intomatplotlib:masterfromNelleV:deprecate_finance

Oct 18, 2016

Merged

MAINT finance module is deprecated#7295

QuLogic merged 2 commits intomatplotlib:masterfromNelleV:deprecate_finance

Oct 18, 2016

Conversation

Copy link

Member

NelleV commentedOct 17, 2016•
edited
Loading

Here is a deprecation for the finance module.

Supersedes#7071
We should probably make a call for contributor out.

MAINT finance module is deprecated

79c6966

mdboom added the status: needs review label

Oct 17, 2016

NelleV changed the title~~MAINT finance module is deprecated~~[MRG] MAINT finance module is deprecated

Oct 17, 2016

tacaswell reviewed

Oct 18, 2016

View reviewed changes

lib/matplotlib/finance.py

		@@ -2,7 +2,7 @@
		A collection of functions for collecting, analyzing and plotting
		financial data. User contributions welcome!

		This module is deprecated in1.4 and will be moved to `mpl_toolkits`
		This module is deprecated in2.0 and will be moved to `mpl_toolkits`

Copy link

Member

Choose a reason for hiding this comment

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

"and has been moved to it's own project" ?

tacaswell changed the title~~[MRG] MAINT finance module is deprecated~~[MRG+1] MAINT finance module is deprecated

Oct 18, 2016

tacaswell added this to the2.0 (style change major release) milestone

Oct 18, 2016

Copy link

Member

tacaswell commentedOct 18, 2016

Looks good modulo minor word smithnig.

Who ever merges, please remember to backport to v2.x

DOC updated finance's docstring

905b695

QuLogic changed the title~~[MRG+1] MAINT finance module is deprecated~~MAINT finance module is deprecated

Oct 18, 2016

QuLogic merged commit33f8412 intomatplotlib:master

Oct 18, 2016

QuLogic removed the status: needs review label

Oct 18, 2016

Copy link

Member

QuLogic commentedOct 18, 2016•
edited
Loading

Backported to v2.x via83292f5.

QuLogic added a commit to QuLogic/matplotlib that referenced this pull request

Oct 18, 2016

Merge pull requestmatplotlib#7295from NelleV/deprecate_finance

83292f5

MAINT finance module is deprecated

Copy link

Contributor

@NelleV Actually this interferes somewhat awkwardly with the fact that IPython still imports pylab (#7186 (comment), fixed as of IPython 5.2). Now you get the deprecation warning when running%matplotlib in IPython...

Copy link

Member

efiring commentedOct 18, 2016

Maybe the deprecation warning needs to be inside each function in the module, instead of at the module level?

Copy link

Member

tacaswell commentedOct 18, 2016

we could just wrap the import in a warning context and snarf the warning?

IPython merged my PR to remove most of the pylab importsipython/ipython#9998

Copy link

Contributor

anntzer commentedOct 18, 2016

I think@tacaswell's solution is the best.

Copy link

MemberAuthor

NelleV commentedOct 18, 2016

I think having the warning at least at module level is important, as some people sometimes import stuff from our modules (totally unrelated to our code) which is going to break as well. (That used to be a reason not to remove unused imports at the top of some of our modules).

I could also be very aggressive and add deprecation warnings to all the functions of the module. That's extremely easy to do with our deprecated decorator.

I'll look at ipython's issue with our deprecation warning.