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.

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

1 internet doctest failing in src/sage/finance/stock.py #32696

Closed
seblabbe opened this issue Oct 14, 2021 · 26 comments
Closed

1 internet doctest failing in src/sage/finance/stock.py #32696

seblabbe opened this issue Oct 14, 2021 · 26 comments

Comments

@seblabbe
Copy link
Contributor

With Ubuntu 18.04 and 9.5.beta3, the command

$ sage -t --optional=sage,internet src/sage/finance/stock.py

gives

Using --optional=internet,sage
Doctesting 1 file.
sage -t --random-seed=0 src/sage/finance/stock.py
**********************************************************************
File "src/sage/finance/stock.py", line 553, in sage.finance.stock.Stock.load_from_file
Failed example:
    finance.Stock('aapl').load_from_file(filename)[:5]
Expected:
    doctest:warning...
    DeprecationWarning: Importing finance from here is deprecated...
    [
    1212408060 188.00 188.00 188.00 188.00        687,
    1212408000 188.00 188.11 188.00 188.00       2877,
    1212407700 188.00 188.00 188.00 188.00       1000,
    1212407640 187.75 188.00 187.75 188.00       2000,
    1212405780 187.80 187.80 187.80 187.80        100
    ]
Got:
    [
    1212408060 188.00 188.00 188.00 188.00        687,
    1212408000 188.00 188.11 188.00 188.00       2877,
    1212407700 188.00 188.00 188.00 188.00       1000,
    1212407640 187.75 188.00 187.75 188.00       2000,
    1212405780 187.80 187.80 187.80 187.80        100
    ]
**********************************************************************
1 item had failures:
   1 of   5 in sage.finance.stock.Stock.load_from_file
    [26 tests, 1 failure, 0.01 s]
----------------------------------------------------------------------
sage -t --random-seed=0 src/sage/finance/stock.py  # 1 doctest failed
----------------------------------------------------------------------

Component: doctest coverage

Author: Sébastien Labbé

Branch/Commit: 9117551

Reviewer: Frédéric Chapoton

Issue created by migration from https://trac.sagemath.org/ticket/32696

@seblabbe seblabbe added this to the sage-9.5 milestone Oct 14, 2021
@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 14, 2021

comment:1

The whole package is deprecated - https://wiki.sagemath.org/ReleaseTours/sage-9.5#Deprecated_and_removed_functionality

@fchapoton
Copy link
Contributor

Author: Frédéric Chapoton

@fchapoton
Copy link
Contributor

Branch: u/chapoton/32696

@fchapoton
Copy link
Contributor

comment:2

please review


New commits:

6b27345fix finance

@fchapoton
Copy link
Contributor

Commit: 6b27345

@seblabbe
Copy link
Contributor Author

comment:3

does not work because "..." does not replace "empty":

sage -t --random-seed=0 src/sage/finance/stock.py
**********************************************************************
File "src/sage/finance/stock.py", line 553, in sage.finance.stock.Stock.load_from_file
Failed example:
    finance.Stock('aapl').load_from_file(filename)[:5]
Expected:
    ...
    [
    1212408060 188.00 188.00 188.00 188.00        687,
    1212408000 188.00 188.11 188.00 188.00       2877,
    1212407700 188.00 188.00 188.00 188.00       1000,
    1212407640 187.75 188.00 187.75 188.00       2000,
    1212405780 187.80 187.80 187.80 187.80        100
    ]
Got:
    [
    1212408060 188.00 188.00 188.00 188.00        687,
    1212408000 188.00 188.11 188.00 188.00       2877,
    1212407700 188.00 188.00 188.00 188.00       1000,
    1212407640 187.75 188.00 187.75 188.00       2000,
    1212405780 187.80 187.80 187.80 187.80        100
    ]
**********************************************************************
1 item had failures:
   1 of   5 in sage.finance.stock.Stock.load_from_file
    [26 tests, 1 failure, 0.01 s]
----------------------------------------------------------------------
sage -t --random-seed=0 src/sage/finance/stock.py  # 1 doctest failed
----------------------------------------------------------------------

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 16, 2021

Changed commit from 6b27345 to 76e5400

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 16, 2021

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

76e5400fix finance

@fchapoton
Copy link
Contributor

comment:5

oh, boy..

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 16, 2021

comment:6

How about just marking this whole file # sage.doctest: optional - deprecated_sage_finance
via #30778, see
https://wiki.sagemath.org/ReleaseTours/sage-9.5#Module-level_.22.23_optional_-_FEATURE.22_for_doctests

@fchapoton
Copy link
Contributor

comment:7

I am leaving this to other hands

@fchapoton
Copy link
Contributor

Changed branch from u/chapoton/32696 to none

@fchapoton
Copy link
Contributor

Changed commit from 76e5400 to none

@fchapoton
Copy link
Contributor

Changed author from Frédéric Chapoton to none

@seblabbe
Copy link
Contributor Author

comment:10

Ok, why not. I added the new global optional tag, but it does not work: the command

$ sage -t --optional=sage,internet src/sage/finance/stock.py

still provide a doctest failure.


New commits:

f222d1e32696:adding global optional tag to deprecated module stock.py

@seblabbe
Copy link
Contributor Author

Commit: f222d1e

@seblabbe
Copy link
Contributor Author

Author: Sébastien Labbé

@seblabbe
Copy link
Contributor Author

Branch: public/32696

@mkoeppe
Copy link
Contributor

mkoeppe commented Oct 22, 2021

comment:11

This needs to go to the very top of the file

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2021

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

911755132696: fix failing doctest

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Oct 28, 2021

Changed commit from f222d1e to 9117551

@seblabbe
Copy link
Contributor Author

comment:13

Replying to @mkoeppe:

This needs to go to the very top of the file

I tried it, but it did not work.

@seblabbe
Copy link
Contributor Author

comment:14

I did a fix like the one proposed by Frederic but by making sure the ... always replaces a non empty string, i.e., containing at least the opening bracket [.

Now, both

sage -t --optional=sage,internet src/sage/finance/stock.py
sage -t src/sage/finance/stock.py

works for me.

Needs review!

@fchapoton
Copy link
Contributor

Reviewer: Frédéric Chapoton

@fchapoton
Copy link
Contributor

comment:16

Allons-y !

@vbraun
Copy link
Member

vbraun commented Nov 18, 2021

Changed branch from public/32696 to 9117551

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

No branches or pull requests

4 participants