ckan / ckanapi

A command line interface and Python module for accessing the CKAN Action API
Other
178 stars 74 forks source link

Fixed Exception clause left in Python 2 style. #96

Closed fccoelho closed 7 years ago

fccoelho commented 8 years ago

Fixed small bug with bytes to str conversion

This commit handle the bugs reported on issue #95

fccoelho commented 8 years ago

I ran the following test here and it worked:

In [7]:  js = {'name': 'São paulo'}

In [8]: str(js)
Out[8]: "{'name': 'São paulo'}"

In [9]: sys.stdout.write(str(js))
Out[9]: {'name': 'São paulo'}21

Can you give an example where it does not?

On the other hand if I follow your suggestions this is what happens:

In [11]: stdout = getattr(sys.stdout, 'buffer', sys.stdout)

In [12]: stdout.write(js)
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-12-f597dc5b97ea> in <module>()
----> 1 stdout.write(js)

TypeError: a bytes-like object is required, not 'dict'

in order to fix it I'd have to convert the object to bytes:

In [14]: stdout.write(str(js).encode())
{'name': 'São paulo'}Out[14]: 22
wardi commented 7 years ago

@fccoelho It would fail in Python 2. str(u'São paulo') raises an exception.

fccoelho commented 7 years ago

Do you still need to support python2?

Anyway, it works as long you omit the u and use a regular string instead of an u'string'

the following works on both Python 2 and 3:

sys.stdout.write(str('São paulo'))
wardi commented 7 years ago

Yes, we need to support Python 2 because that's required for -c (running the api calls locally directly on a ckan config file)

It looks like action() does yield byte strings, so then this change is actually a no-op in python 2 but in Python 3 if you call str() on a byte string you'll get the repr of what's returned, which isn't valid JSON.

timwis commented 7 years ago

Hi folks, any update on this? Just ran into the TypeError: write() argument must be str, not bytes issue as well using python3 and ckanapi v4.

wardi commented 7 years ago

fixed by fa920b2