-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Remove finance module #7071
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
Remove finance module #7071
Conversation
This probably need an api change note |
This is probably worth a note to the user and devel lists one more time. This is also going to require coordination with the major down-stream packagers (fedora, debian, arch, enthought, contiuum, homebrew, macports, pythonxy, winpython) that they may need to create a new package. I am 50/50 on backporting to 2.0. No compelling argument against other than making sure there is enough time for the transition to go smoothly. Can definitely be talked into it. |
I honestly hoped that we could avoid waisting to many package maintainers time on this package since it's a pure python package with only one file but we should probably send them a heads up anyway. My main reason to suggest 2.0 was that we already backported the removal of the examples.
|
I agree with @tacaswell that we may want to wait until 2.1 and not backporting 2.0 to make the transition smoothly. |
In my opinion that would be the very definition of non smooth. You were not willing to disable the examples so that they could be removed along with the module so they have already been removed from 2.0 thus the module should be removed along with the examples anything else just raises extra confusion for all users |
Right now there is no deprecation warning when importing the finance module. The documentation examples (FYI, mostly unrelated to the finance module) do not need to follow the same deprecation cycle, as no users uses those as dependencies. In additions the examples were not running… If they hadn't caused any problem, I wouldn't have encouraged removing them (note that api.yahoo is still either down or insanely slow). |
The examples were running and still are. Travis ran them 6 hours before your issue and I ran them again when I created the new package afterwards. I guess they failed for you due to a temporary issue on yahoo's site or a proxy issue |
or more likely your have been blocked by a rate limiter on the yahoo api since you have been rebuilding over and over again |
Your interpretation on why we couldn't build the documentation in Reading anymore is probably correct. Considering I had to rebuild the documentation many times in order to have the list of dependencies, that may have been a problem. It is surprising that no one else had this problem before: I only rebuild the documentation around 20 times, which is a reasonable amount of time for anyone working on the documentation. Anyhow, that doesn't changes the fact that removing the finance module should go through a proper deprecation cycle. It should raise a Deprecation Warning for at least two releases before being removed. Right now, it is only documented in the docstring. It is unlikely anyone noticed this deprecation. |
This module is deprecated in 1.4 and will be moved to `mpl_toolkits` | ||
or it's own project in the future. | ||
|
||
Shim to provide backword compatibility for finance |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"backward"
Given the lack of agreement, perhaps we can just noisily deprecate this module for 2.0 and remove it (without introducing an optional dependency on mpl_finance -- let's just get rid of it) in 2.1? With some luck(?) someone who actually uses this module (which seems of interest for none of us) will notice the warning, come forward and propose to maintain it, possibly somewhere else. @NelleV I don't know why you'd want a deprecation cycle of at least two releases. Since 1.1 mpl has averaged one release (x.y) per year (in fact I just noticed that each 1.x.0 release, except for 1.0, occured in year 201x :-)) so that seems a ridiculously long cycle... |
@anntzer Because that's the norm. It is normal to have long deprecation cycles for a project of the size of matplotlib where there are tens of thousands of users. Some would even argue that you should just not break backward compatibility. Second, maybe we should try to have shorter release cycles :) |
I think this kind of deprecation policy is a bit misguided; at the end of the day what matters most for the end user is not the number of minor releases but the physical duration of the deprecation cycle. From his point of view, it's basically "let's run this code, as usual... oh there's a deprecation warning, I need to either update the code, or to stick everything in a venv/conda env with pinned dependencies". (Additionally, these days conda envs are really easy to set up. Back when this was not an option, I could understand the need to keep things more stable.) The duration of the deprecation cycle basically tells you how much time he has to do this. Honestly, if a year (or six months, assuming we move to a faster release cycle, which I don't feel strongly about at all) are not enough for someone to move his codebase into a conda env, I don't see how givin him two years will change anything. One thing I do agree, though, is that you do want at least one cycle of noisy deprecation. (As a side note, I remember ranting with you about how I hated that the IPython project (pre big-split) was often breaking their (semi-internal, I guess) API -- but I'd say that was before conda envs were really common, and due to the lack of noisy deprecations.) In fact we could even make the deprecation message more explicit: "Due to lack of active maintenance, the XYZ module is scheduled for removal in a future version. Interested parties are invited to step forward to maintain the module." Then we'll see if anyone actually cares... I know very well that the 2.0 cycle is rediculously long because of the many difficulties related to the default style change. I don't actually mind the relatively slow release cycle -- I am happy that things are done with the proper level of attention for details. |
@anntzer When I wrote my phd thesis, I ran code that I wrote 3 years before that and hadn't touched since. 1 year is not that common. Some people don't use Conda. To quote a visiting scholar from BIDS, "apart in the scientific python community, I don't know anyone who uses conda". More exactly, he said that before coming here to visit the Berkeley Institute for data science, he had never heard of conda. Anyhow, I don't feel strongly about the 2 deprecation cycle, thought I do think that strong backward compatibility is what makes matplotlib such a widely used package. |
But then your position amounts to saying that we should never deprecate anything, because even a one-year (or, in your case, three-year!) deprecation cycle can catch people unprepared. For example, even though we are trying as hard as possible to make 2.0 "backwards-compatible" in the sense that code that ran on 1.5 will still produce a somewhat similar plot on 2.0, I think it's clear that many plots will require some amount of tuning to stay aesthetically pleasing. And there's nothing we can do about it... Does that mean that, because you wanted your three-year old code to run unmodified after the switch to 2.0, we cannot make that change? Now, I don't know what is the actual status of the finance module in terms of code quality, but I don't think that keeping half (and I'm being generous there)-working code in the codebase such as mpl_toolkits.gtktools (#6554) or some parts of mlab (#6577) is helpful for anyone either. |
I am saying these things should not be taken lightly. |
I am slightly confused on how a two release cycle of deprecation bothers you though. It's not like its code you'll have to maintain. Before we tagged which version we removed deprecated stuff on sklearn to easily grep them and remove them before the release, some functions were deprecated for a very long time: no one noticed. |
I agree code cannot be removed randomly and that proper care is needed. In fact that's exactly why I proposed a middle ground: deprecate in 2.0 (I agree with you that the deprecation-in-doc-only of the finance module is insufficient), and remove after that. It's not code I have to maintain, but as long as it's there, people will notice issues, open PRs, etc. and energy will be lost on that (even though I agree it's not that much). In fact, searching for "finance" in the issues led me to your own comment (#7057 (comment)):
:-) |
Seems like #7295 took care of issuing a deprecation warning, so this won't be removed until 2.2 - Close me, maybe? ;) |
And replace with a shim using mpl_finance. I would suggest backporting to 2.0