Opened 6 years ago

Closed 5 years ago

Last modified 4 years ago

#13884 closed defect (fixed)

Fix optional internet tests.

Reported by: robertwb Owned by: mvngu
Priority: major Milestone: sage-6.2
Component: doctest coverage Keywords:
Cc: Merged in:
Authors: Karl-Dieter Crisman Reviewers: Ralf Stephan, Volker Braun
Report Upstream: N/A Work issues:
Branch: 90a4128 (Commits) Commit:
Dependencies: Stopgaps:

Description (last modified by chapoton)

Untested = broken.

sage -t --only_optional=internet devel/sage/sage/symbolic/integration/integral.py # 2 doctests failed
sage -t --only_optional=internet devel/sage/sage/combinat/words/paths.py # 1 doctests failed
sage -t --only_optional=internet devel/sage/sage/misc/preparser.py # 1 doctests failed
sage -t --only_optional=internet devel/sage/sage/interfaces/r.py # 1 doctests failed
sage -t --only_optional=internet devel/sage/sage/finance/stock.py # 11 doctests failed
sage -t --only_optional=internet devel/sage/sage/databases/sloane.py # 8 doctests failed

The files sage/databases/sloane.py and sage/combinat/words/paths.py are really covered by #10358.

The file sage/interfaces/r.py is a situation needing clarification at #7771.

This ticket is to fix the minor issues in the integral, preparer, and stock files.

Apply:

Attachments (2)

trac_13884.patch (9.0 KB) - added by kcrisman 6 years ago.
trac_13884_addon1.patch (4.5 KB) - added by chapoton 6 years ago.

Download all attachments as: .zip

Change History (26)

comment:1 Changed 6 years ago by robertwb

So this doesn't happen again, re-enable these via #13540.

comment:2 Changed 6 years ago by kcrisman

sage -t --only_optional=internet devel/sage/sage/interfaces/r.py

Just for clarification, this one is broken, but not because of the internet, and has been for a while. This was originally noticed (and fixed) by Minh while reviewing #7771 in this patch. It wasn't clear how to proceed, as you can see by reading the comments there.

comment:3 Changed 6 years ago by kcrisman

Haha! And the first one is not really a problem, just a logic issue.

sage -t --only_optional=internet devel/sage/sage/symbolic/integration/integral.py
    sage: f.integrate(algorithm="mathematica_free")       # optional -- requires internet
Expected:
    sqrt(pi)*sqrt(1/2)*fresnels(sqrt(2)*x/sqrt(pi)) + y^z*x
Got:
    doctest:674: DeprecationWarning: Variable of integration should be specified explicitly.
    See http://trac.sagemath.org/12438 for details.
    sqrt(pi)*sqrt(1/2)*fresnels(sqrt(2)*x/sqrt(pi)) + y^z*x
**********************************************************************
Expected:
    doctest:...: DeprecationWarning:
    Variable of integration should be specified explicitly.
    See http://trac.sagemath.org/12438 for details.
    1/2*x^2
Got:
    1/2*x^2

What happens is that when the test is done, the deprecation warning, which only occurs once, occurs with the internet test. This is a two-character fix. I'll try to see if I can fix any others along with it.

comment:4 Changed 6 years ago by kcrisman

Next: Considering that William at some point changed the file in this test

    sage: sage.misc.preparser.load('http://wstein.org/loadtest.py', globals())  # optional - internet
Expected:
    hi from the net
Got:
    hi from the net
    5

to

print "hi from the net"

print 2+3      

I think we'll be okay just changing this one too.

comment:5 Changed 6 years ago by kcrisman

Just for reference, in general this file (sloane.py) is almost completely undocumented and #10358 is supposed to update with respect to this. So this is fully known.

Last edited 6 years ago by kcrisman (previous) (diff)

comment:6 Changed 6 years ago by kcrisman

The finance errors seem mostly due to a change in how Google uses startdate, though at least one of them shouldn't have even worked the first time!

    sage: finance.Stock('vmw').google()[:5]   # optional -- internet
Expected:
    [
     28-Nov-07 80.57 88.49 80.57 87.69    7496000,
     29-Nov-07 90.91 93.20 89.50 90.85    5497600,
     30-Nov-07 95.39 95.60 89.85 91.37    4750200,
      3-Dec-07 89.87 96.00 88.70 94.97    4401100,
      4-Dec-07 92.26 97.10 92.05 95.08    2896600
    ]
Got:
    [
      5-Jan-12 81.76 82.84 81.52 82.31    2888520,
      6-Jan-12 82.96 84.33 82.47 83.31    1922781,
      9-Jan-12 83.45 85.17 82.94 84.95    1841373,
     10-Jan-12 86.01 86.80 85.10 85.30    1762911,
     11-Jan-12 85.42 88.20 85.30 87.92    2192664
    ]

At least this tells us when William added the finance stuff!

comment:7 Changed 6 years ago by kcrisman

And finally, the combinat error is due to the broken sloane_find issue. So this ticket should be quite fixable - hope to have a patch up soon.

comment:8 Changed 6 years ago by kcrisman

  • Description modified (diff)

comment:9 Changed 6 years ago by kcrisman

Google now apparently does not return data that is more than a certain number of years old - or at least the interface must have changed?

sage: finance.Stock('F').google('Jan+3,+1978', 'Jul+7,+2008')[:1]
[
 20-Aug-92 0.00 2.26 2.21 2.24   19202479
]
sage: finance.Stock('F').google('Jan+3,+1982', 'Jul+7,+2008')[:1]
[
 20-Aug-92 0.00 2.26 2.21 2.24   19202479
]

In fact, according to this blog post,

These APIs are now deprecated but have no scheduled shutdown date: ... Finance API ...

And although the data is still there if one uses e.g. a URL like this downloading the CSV only gives you so much.

What should we do, oh many Sage devels who are Google employees? ;-) I can fix some stuff but don't want to fix more of this if it's going to be gone eventually anyway.

comment:10 Changed 6 years ago by kcrisman

Okay, the following patch fixes everything not from #10358, #7771, and the change in the CSV files that the Google Finance API produces. (Note that Apple must have had a 4-1 stock split at some point as well.) In some cases I could circumvent the latter when the point wasn't testing historical data, but there are still two failures.

sage -t --only_optional=internet "devel/sage/sage/finance/stock.py"
**********************************************************************
    sage: finance.Stock('F').google('Jan+3,+1978', 'Jul+7,+2008')[:5] # optional -- internet
Expected:
    [
      3-Jan-78 0.00 1.93 1.89 1.89    1618200,
      4-Jan-78 0.00 1.89 1.87 1.88    2482700,
      5-Jan-78 0.00 1.89 1.84 1.84    2994900,
      6-Jan-78 0.00 1.84 1.82 1.83    3042500,
      9-Jan-78 0.00 1.81 1.79 1.81    3916400
    ]
Got:
    [
     20-Aug-92 0.00 2.26 2.21 2.24   19202479,
     21-Aug-92 0.00 2.27 2.19 2.20   18689616,
     24-Aug-92 0.00 2.17 2.10 2.10   38652825,
     25-Aug-92 0.00 2.19 2.11 2.18   31028049,
     26-Aug-92 0.00 2.21 2.18 2.20   22539411
    ]
**********************************************************************
    sage: sage.finance.stock.Stock("AAPL", 22144).google(startdate='Jan+1,+1990')[:5] #optional -- internet
Expected:
    [
      2-Jan-90 0.00 9.38 8.75 9.31    6542800,
      3-Jan-90 0.00 9.50 9.38 9.38    7428400,
      4-Jan-90 0.00 9.69 9.31 9.41    7911200,
      5-Jan-90 0.00 9.56 9.25 9.44    4404000,
      8-Jan-90 0.00 9.50 9.25 9.50    3627600
    ]
Got:
    [
     28-Feb-97 0.00 1.05 1.02 1.02   17411200,
      3-Mar-97 0.00 1.03 1.00 1.01   18636800,
      4-Mar-97 0.00 1.03 1.00 1.03   14745600,
      5-Mar-97 0.00 1.06 1.03 1.06   13737600,
      6-Mar-97 0.00 1.06 1.03 1.04   16612800
    ]
**********************************************************************

I don't know if these are fixable.

Changed 6 years ago by kcrisman

comment:11 Changed 6 years ago by kcrisman

  • Authors set to Karl-Dieter Crisman
  • Description modified (diff)
  • Status changed from new to needs_review

Patchbot, apply trac_13884.patch

I'd appreciate feedback; however, the easiest to me seems to be to review this and then open a new ticket to fix the Google finance interface csv download issue - if that is even possible.

comment:12 Changed 6 years ago by kcrisman

  • Status changed from needs_review to needs_work

Okay, in the meantime the Google stuff has changed yet again (???), and we get the following bizarre error:

File "sage/symbolic/integration/integral.py", line 466, in sage.symbolic.integration.integral.integrate
Failed example:
    f.integrate(algorithm="mathematica_free")       # optional - internet
    NameError: name 'f' is not defined
**********************************************************************

This one baffles me; even doing f.integrate(x) with no algorithm or anything still fails. Doing this by hand usually works - I think there is some potential for timeouts. But it consistently fails with this in doctesting. So I'm giving up on this patch for now, unless someone has some other bright ideas.

comment:13 Changed 6 years ago by jdemeyer

  • Milestone changed from sage-5.11 to sage-5.12

comment:14 Changed 6 years ago by chapoton

  • Description modified (diff)

Changed 6 years ago by chapoton

comment:15 Changed 6 years ago by chapoton

  • Description modified (diff)

I think this solves the problem in the integral file.

comment:16 Changed 6 years ago by kcrisman

The add-on patch is good. I'm not sure why that should make a difference, but it did.

I'm tempted to have us just add the preparser test to the integral one and then open a new ticket for stock.py, which clearly isn't functioning properly in any case (not just a problem of changed values any more). Frederic, do you want to create a patch like that?

comment:17 Changed 5 years ago by vbraun

A partial fix is better than no fix at all, IMHO.

comment:18 Changed 5 years ago by robertwb

I agree, no need to fix all of them before any of them go in.

comment:19 Changed 5 years ago by chapoton

  • Branch set to u/chapoton/13884
  • Commit set to 90a4128e8e2eebf6f79511d684b56c1c710464dd
  • Status changed from needs_work to needs_review

New commits:

90a4128Trac #13884 - fix most broken internet optional doctests

comment:20 Changed 5 years ago by vbraun_spam

  • Milestone changed from sage-6.1 to sage-6.2

comment:21 Changed 5 years ago by rws

What test command do you use? With --optional=internet I get:

sage -t src/sage/finance/stock.py  # 11 doctests failed
sage -t src/sage/symbolic/integration/external.py  # 1 doctest failed

comment:22 Changed 5 years ago by vbraun

  • Reviewers set to Ralf Stephan, Volker Braun
  • Status changed from needs_review to positive_review

Sounds good to me; somebody interested in finance can take up the stock.py failures.

comment:23 Changed 5 years ago by vbraun

  • Branch changed from u/chapoton/13884 to 90a4128e8e2eebf6f79511d684b56c1c710464dd
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:24 Changed 4 years ago by chapoton

  • Commit 90a4128e8e2eebf6f79511d684b56c1c710464dd deleted
  • Description modified (diff)
Note: See TracTickets for help on using tickets.