`ImportError: No module named pathlib` and wrong exception

[42B]

1

117 introduced a bug in regards to Python 2 compatibility since pathlib is Python 3.4+ only.

Python 2.7.15rc1 (default, Nov 12 2018, 14:31:15) 
[GCC 7.3.0] on linux2
>>> import quandl
>>> quandl.read_key()
Traceback (most recent call last):
...
ImportError: No module named pathlib
>>> quandl.save_key('secret...')
Traceback (most recent call last):
...
ImportError: No module named pathlib

Currently, the only way to hit these import statements is to not pass a filename argument to either function (or pass filename=None). Here are the two sections causing the issue:

https://github.com/quandl/quandl-python/blob/33149816637b239db6a6436cc3d4a2d5a60d5fe4/quandl/api_config.py#L15-L18

https://github.com/quandl/quandl-python/blob/33149816637b239db6a6436cc3d4a2d5a60d5fe4/quandl/api_config.py#L26-L29

This problem would likely have been caught earlier if the imports were at the top of the file as mentioned in PEP8.

https://github.com/quandl/quandl-python/blob/33149816637b239db6a6436cc3d4a2d5a60d5fe4/quandl/__init__.py#L3

A simple fix seems to be indicated in the Python docs themselves:

Return a new path object representing the user’s home directory (as returned by os.path.expanduser() with ~ construct)

• https://docs.python.org/3/library/pathlib.html#pathlib.Path.home
• https://docs.python.org/3/library/pathlib.html#correspondence-to-tools-in-the-os-module

Python 3.7.1 (default, Oct 25 2018, 21:25:21) 
[GCC 7.3.0] on linux
>>> import os
>>> import pathlib
>>> os.path.expanduser('~') == str(pathlib.Path.home())
True

There is a problem with simply replacing str(pathlib.Path.home()) with os.path.expanduser('~') in these two lines though. The string concatenation is prepending forward slash in front of .quandl_apikey which (if I remember correctly) is not compatible with Windows which uses backslashes.

filename = str(pathlib.Path.home()) + "/.quandl_apikey" 
                                       ^

I'm a fan of pathlib and one way to solve this would be like this:

>>> pathlib.Path.home() / ".quandl_apikey"
PosixPath('/home/.../.quandl_apikey')

But since Python 2 compatibility is the issue, using os.path.join seems like an appropriate fix.

>>> p = pathlib.Path.home() / ".quandl_apikey"
>>> str(p) == os.path.join(os.path.expanduser('~'), '.quandl_apikey')
True

---

2

There is also a smaller bug where the try/except in read_key is looking for the wrong exception. In Python 2, open raises an IOError if a file isn't found. In Python 3, an OSError is raised.

Neither of these exceptions have a parent exception of ValueError though.

• https://docs.python.org/2/library/exceptions.html#exception-hierarchy
• https://docs.python.org/3/library/exceptions.html#exception-hierarchy

Python 2.7.15rc1 (default, Nov 12 2018, 14:31:15) 
[GCC 7.3.0] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> try:
...     with open('doesnt_exist.txt', 'r') as f:
...         pass
... except ValueError:
...     raise Exception("File '{:s}' not found.".format(filename))
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
IOError: [Errno 2] No such file or directory: 'doesnt_exist.txt'

Python 3.7.1 (default, Oct 25 2018, 21:25:21) 
[GCC 7.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> try:
...     with open('doesnt_exist.txt', 'r') as f:
...         pass
... except ValueError:
...     raise Exception("File '{:s}' not found.".format(filename))
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
FileNotFoundError: [Errno 2] No such file or directory: 'doesnt_exist.txt'

In my opinion, these exceptions are already very clear about what went wrong and it seems unnecessary to catch them in read_key just to simply re-raise them with a custom message.

---

tl;dr

Ignoring the fact that these two functions are mutating the state of a class and seem like they could be class methods instead... maybe something like this?

import os

class ApiConfig:
    api_key = None
    api_protocol = 'https://'
    api_base = '{}www.quandl.com/api/v3'.format(api_protocol)
    api_version = None
    page_limit = 100

    use_retries = True
    number_of_retries = 5
    retry_backoff_factor = 0.5
    max_wait_between_retries = 8
    retry_status_codes = [429] + list(range(500, 512))

def save_key(apikey, filename=None):
    if filename is None:
        filename = os.path.join(os.path.expanduser('~'), '.quandl_apikey')

    with open(filename, 'w') as f:
        f.write(apikey)

    ApiConfig.api_key = apikey

def read_key(filename=None):
    if filename is None:
        filename = os.path.join(os.path.expanduser('~'), '.quandl_apikey')

    with open(filename, 'r') as f:
        apikey = f.read()

    if not apikey:
        raise ValueError("File '{:s}' is empty.".format(filename))

    ApiConfig.api_key = apikey

[A-Scott-Rowe]

Hi @42B ,

Both of these fixes seems totally reasonable Can you open a PR with the changes rebased against our master so we can get them into the codebase?

[A-Scott-Rowe]

Adressed in https://github.com/quandl/quandl-python/pull/131. Closing