py3: minor cleanup in sage.finance.stock

[fchapoton]

CC: @embray @jdemeyer

Component: python3

Author: Frédéric Chapoton

Branch/Commit: d73a9fe

Reviewer: Vincent Klein

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

[fchapoton]

Branch: u/chapoton/26213

[fchapoton]

New commits:

172458e

py3: fix doctests in finance

[fchapoton]

Commit: 172458e

[vinklein]

comment:2

The two tests use the doctest tag # py2. Therefore it fails with python2.

[vinklein]

Reviewer: Vincent Klein

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

a6918b2

fix

[sagetrac-git]

Changed commit from 172458e to a6918b2

[fchapoton]

comment:5

thanks, fixed

[vinklein]

comment:6

With python3, i get :

sage -t src/sage/finance/stock.py
**********************************************************************
File "src/sage/finance/stock.py", line 573, in sage.finance.stock.Stock.load_from_file
Failed example:
    finance.Stock("AAPL").load_from_file("I am not a file") # py3
Expected:
    Traceback (most recent call last):
    ...
    FileNotFoundError: [Errno 2] No such file or directory: 'I am not a file'
Got:
    <BLANKLINE>
    Traceback (most recent call last):
      File "/home/vklein/odk/sage/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 650, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/vklein/odk/sage/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 1061, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.finance.stock.Stock.load_from_file[4]>", line 1, in <module>
        finance.Stock("AAPL").load_from_file("I am not a file") # py3
      File "/home/vklein/odk/sage/local/lib/python3.6/site-packages/sage/finance/stock.py", line 578, in load_from_file
        file_obj = open(file, 'r')
    FileNotFoundError: [Errno 2] No such file or directory: 'I am not a file'

And if i add a <BLANKLINE> in the doctest expected result :

sage -t src/sage/finance/stock.py
**********************************************************************
File "src/sage/finance/stock.py", line 573, in sage.finance.stock.Stock.load_from_file
Failed example:
    finance.Stock("AAPL").load_from_file("I am not a file") # py3
Exception raised:
    Traceback (most recent call last):
      File "/home/vklein/odk/sage/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 650, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/vklein/odk/sage/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 1061, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.finance.stock.Stock.load_from_file[4]>", line 1, in <module>
        finance.Stock("AAPL").load_from_file("I am not a file") # py3
      File "/home/vklein/odk/sage/local/lib/python3.6/site-packages/sage/finance/stock.py", line 579, in load_from_file
        file_obj = open(file, 'r')
    FileNotFoundError: [Errno 2] No such file or directory: 'I am not a file'

It behave as if the Error is not catched by the doctest framework.

[fchapoton]

comment:7

Then let us ask for help.. Erik or Jeroen, any idea ?

[vinklein]

comment:8

To simplify i have tested the following doctest :

sage: raise NameError("test")
Traceback (most recent call last):
...
NameError: test
sage: raise FileNotFoundError("test")
Traceback (most recent call last):
...
FileNotFoundError: test

The first test pass and the second fails.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

e462f1a

fix for py3

[sagetrac-git]

Changed commit from a6918b2 to e462f1a

[fchapoton]

comment:10

ok. So the reason is that there is in sage doctesting framework a mechanism that modifies some subexceptions of OSError in python3, and replace them with OSError.

Should be good to go now. Unless Erik (@embray) has a better idea ?

[vinklein]

comment:11

Indeed, it's the #25676 ticket.

[vinklein]

comment:13

It's ok for me. I let Eric pass it in positive review.

[embray]

comment:14

Because of #25676 this shouldn't require separate #py2 and #py3 cases at all, unless I missed a case in #25676.

[fchapoton]

comment:15

Well, I tried and it did not work. Because you are busy on other stuff, I thought it was good enough for the moment.

[embray]

comment:16

This is pretty low priority though (I would argue even trivial) so I don't see any reason to rush to a fix. I can have a look at it.

[embray]

comment:17

Weird. I don't even have that test locally. Was it just added in 8.4.beta4?

[fchapoton]

comment:18

last modified seven years ago..

[embray]

comment:19

Oh I see now. In my branch I just removed that test completely because it was pointless. All it's testing is that the open(...) built-in raises an exception if the given file does not exist (duh). There's nothing in that code that even does anything special in that case, so the test is not useful at all.

[embray]

comment:20

I have some other minor cleanup in that file as well, such as replacing open() with with open(). I think I missed the spurious semicolon you had. This is my diff to the file:

diff --git a/src/sage/finance/stock.py b/src/sage/finance/stock.py
index 1c9ba47..dfab025 100644
--- a/src/sage/finance/stock.py
+++ b/src/sage/finance/stock.py
@@ -124,7 +124,7 @@ class OHLC:
             True
         """
         return not (self == other)
-
+

 class Stock:
     """
@@ -562,18 +562,11 @@ class Stock:
             1212407640 187.75 188.00 187.75 188.00       2000,
             1212405780 187.80 187.80 187.80 187.80        100
             ]
-
-        This tests a file that doesn't exist::
-
-            sage: finance.Stock("AAPL").load_from_file("I am not a file")
-            Traceback (most recent call last):
-            ...
-            IOError: [Errno 2] No such file or directory: 'I am not a file'
         """
-        file_obj = open(file, 'r')
-        R = file_obj.read();
+        with open(file, 'r') as fobj:
+            R = fobj.read()
+
         self.__historical = self._load_from_csv(R)
-        file_obj.close()
         return self.__historical

Still weird that it's failing for you though. I might temporarily restore the test just to investigate a little. But I still think it should just be deleted entirely.

[embray]

comment:21

For that matter the 'r' argument to open() is superfluous as well.

[embray]

comment:22

Ah, I see the problem. There are two classes of exception name mappings surrounding OSError: One is classes in Python 2 that have just been replaced with OSError (of which IOError happens to be one). The other is new-to-Python 3 specialized subclasses of OSError for specific errnos, such as FileNotFoundError.

On Python 3 the doctester code treats either one case, or the other. But in this case it should treat both by mapping IOError -> OSError -> FileNotFoundError. I'll make a ticket.

[fchapoton]

comment:23

ok. Let us remove the useless doctest here.

[sagetrac-git]

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

d73a9fe

py3: fix finance doctest

[sagetrac-git]

Changed commit from e462f1a to d73a9fe

[fchapoton]

comment:25

done, ready for review again

[embray]

comment:27

In case it helps Volker.

[vbraun]

Changed branch from u/chapoton/26213 to d73a9fe