bunq / sdk_python

Python SDK for bunq API
MIT License
106 stars 25 forks source link

ImportError: cannot import name 'JSONDecodeError' #72

Closed jeroenmeulenaar closed 6 years ago

jeroenmeulenaar commented 6 years ago

Steps to reproduce:

What happens:

~ $ python3 test.py
Traceback (most recent call last):
  File "test.py", line 1, in <module>
    from bunq.sdk import context
  File "/usr/local/lib/python3.4/dist-packages/bunq/sdk/context.py", line 6, in <module>
    from bunq.sdk.model import core
  File "/usr/local/lib/python3.4/dist-packages/bunq/sdk/model/core.py", line 1, in <module>
    from bunq.sdk import client
  File "/usr/local/lib/python3.4/dist-packages/bunq/sdk/client.py", line 2, in <module>
    from json import JSONDecodeError
ImportError: cannot import name 'JSONDecodeError'

SDK version and environment

needs #78

OGKevin commented 6 years ago

@jeroenmeulenaar Do you mind upgrading to:

python --version
Python 3.6.3

As I could not reproduce this on 3.6.3 . And all tests are passing as well with this import statement as you can see here: https://github.com/bunq/sdk_python/blob/3ba4987c954cc66f2bbfc67b5e497d951041d53a/tests/bunq_test.py#L5

So, if this has to do with python 3.4.2, then we must either increase the minimum required python version, or find out why it doesn't work on that version.

jeroenmeulenaar commented 6 years ago

@OGKevin I'd like not to upgrade, as I like to stay on Debian stable.

I think I can workaround this by changing the offending import

https://github.com/bunq/sdk_python/blob/3ba4987c954cc66f2bbfc67b5e497d951041d53a/bunq/sdk/client.py#L2

to

from simplejson import JSONDecodeError

I don't know enough about the differences between json and simplejson to know if this is okay...

I also run into a new issue: #73. If it's really needed I can look into upgrading python, but I'd like to understand it first...

OGKevin commented 6 years ago

@jeroenmeulenaar I could reproduce this indeed on python 3.4.2 I have a felling that to solve this we would need to restructure a majority of the project 🤦‍♂️.

So we have 2 options, increase the minimum required python version or see if we can make it backward compatible.

I think if we dive into the change logs of python we could find out why it works on 3.6 and not on 3.4.

Cant you use pyenv on debian stable ?

jeroenmeulenaar commented 6 years ago

@OGKevin Here is the original python issue: https://bugs.python.org/issue19361, from https://docs.python.org/3/whatsnew/3.5.html.

I think it could it be solved by changing https://github.com/bunq/sdk_python/blob/3ba4987c954cc66f2bbfc67b5e497d951041d53a/bunq/sdk/client.py#L254 to except ValueError, since JSONDecodeError is a subclass of ValueError. It doesn't look like you use the added functionality of JSONDecodeError anyway :)

And yes, I could workaround it (pyenv, or making my own fork), but maybe others hit this issue as well. Debian stable is not very up-to-date, but it's very stable :)

OGKevin commented 6 years ago

@jeroenmeulenaar I see, do you mind making a PR if you think you have fixed it ? If not ill look into it, but that will take some time I have a lot on my list ATM. We will include it in the 0.13.0 release