FIX: Removed financial demos that stalled because of yahoo requests #7057

Merged
merged 3 commits into from Sep 8, 2016

Conversation

Projects
None yet
6 participants
Contributor

rougier commented Sep 7, 2016 edited

This PR removes 5 examples from the documenation because they are relying on financial data from yahoo that do not seem to be available anymore. see #7049

@rougier rougier FIX: Removed financial demos that stalled because of yahoo requests t…
…hat never end (timeout way too long)
272e15b

mdboom added the needs_review label Sep 7, 2016

Owner

jenshnielsen commented Sep 7, 2016

The examples are being build on travis (as late as 6 hours ago https://travis-ci.org/matplotlib/matplotlib/builds/158107836) so I think this is a temporary outage.

I would suggest just disabling them from running as parts of the docs build as suggested in #7049

Contributor

NelleV commented Sep 7, 2016

@jenshnielsen I think we should remove them:

  1. This are in git history. They are not lost.
  2. The finance module is already deprecated.
  3. That code is terrible in so many way. We could fix it, by adding a timeout, but then we would need to disable the fact that warning raises errors in the build of our documentation. I personally think it is not worth it considering the code is already deprecated. It also means that people can't work offline (!!).
  4. We also need to stop keeping code stacked, that no one uses, as it leads to situation were we have code that doesn't run, without anyone noticing for years and years and years.
Contributor

rougier commented Sep 7, 2016

  1. Also any change in the yahoo API means the documentation is broken.
Owner

jenshnielsen commented Sep 7, 2016

It may be that the finance module is already deprecated but we can't remove it unless we want lots of complaints from users that are still depending on it. I am just suggesting that we:

  1. Disable all the finance examples from the docs now.
  2. Move the finance module and all it's examples to it's own python package and delete it from matplotlib
Member

Kojoley commented Sep 7, 2016

I think the data used in the examples can be saved to mpl repository and used to 'proxify' urllib requests. It can also be used in the tests for financial module

Kojoley closed this Sep 7, 2016

mdboom removed the needs_review label Sep 7, 2016

Member

Kojoley commented Sep 7, 2016

Oh sorry for closing. This was a mistake.

Kojoley reopened this Sep 7, 2016

mdboom added the needs_review label Sep 7, 2016

Contributor

rougier commented Sep 7, 2016

Why not generate totally fake data instead, would this make a difference for the example ?

Owner

jenshnielsen commented Sep 7, 2016

I agree that it's not a good use of time to do any work on these examples to fix the data loading then I would rather delete them, I just suggest that they should be deleted when they are moved with the finance code to their own repository. Then if anyone should feel like improving the code and or the examples they can do so in that repository independent of matplotlib.

Member

Kojoley commented Sep 7, 2016

Why not generate totally fake data instead, would this make a difference for the example ?

There is fetch_historical_yahoo I think examples should be rewritten to use it and in that way cachename parameter can be used for this

Owner

jenshnielsen commented Sep 7, 2016

I will try to get around to creating a mpl_finance package asap that way we can delete the examples right away and perhaps even the finace module in time for 2.0

Owner

tacaswell commented Sep 7, 2016

We probably should shim out mpl.finance to import from the new package and raise an exception with instructions of what to install to fix it.

Owner

tacaswell commented Sep 7, 2016

To keep things moving for @NelleV and company I am going to merge this and then immediantly open a PR to revert the merge. I think this unblock current progress and make sure we do not (will make it less likely) that these fall through the cracks.

Owner

tacaswell commented Sep 7, 2016

Just waiting for the docs to finish on travis to make sure we do not have an explicit reference to any of these.

Owner

tacaswell commented Sep 7, 2016

There seems to be a hard-coded list someplace

/home/travis/build/matplotlib/matplotlib/venv/lib/python3.5/site-packages/numpy/ma/core.py:4144: UserWarning: Warning: converting a masked element to nan.
  warnings.warn("Warning: converting a masked element to nan.")
Exception occurred:
  File "/home/travis/build/matplotlib/matplotlib/venv/lib/python3.5/site-packages/sphinx/util/parallel.py", line 99, in _join_one
    raise SphinxParallelError(*result)
sphinx.errors.SphinxParallelError: FileNotFoundError: [Errno 2] No such file or directory: '/home/travis/build/matplotlib/matplotlib/doc/mpl_examples/pylab_examples/finance_work2.py'
The full traceback has been saved in /tmp/sphinx-err-jsfn96ab.log, if you want to report the issue to the developers.

rougier added some commits Sep 7, 2016

@rougier rougier Removed financial screenshosts
9192204
@rougier rougier Remove reference to the finance demo
2a5017a
Owner

jenshnielsen commented Sep 8, 2016

Minimal mpl_finance package up at https://github.com/matplotlib/mpl_finance

@tacaswell tacaswell merged commit dcab32b into matplotlib:master Sep 8, 2016

2 of 3 checks passed

continuous-integration/appveyor/pr Waiting for AppVeyor build to complete
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.0002%) to 70.362%
Details

tacaswell removed the needs_review label Sep 8, 2016

@tacaswell tacaswell added a commit that referenced this pull request Sep 8, 2016

@tacaswell tacaswell Merge pull request #7057 from rougier/bad_finance
DOC: Removed financial demos that stalled because of yahoo requests
b152ff9
Owner

tacaswell commented Sep 8, 2016

backported to v2.x as b152ff9

Contributor

NelleV commented Sep 8, 2016

\o/ Thanks!

rougier deleted the rougier:bad_finance branch Sep 9, 2016

anntzer referenced this pull request Oct 16, 2016

Closed

Remove finance module #7071

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment