Two test failures when ujson installed

[jayvdb]

When ujson is installed, there are two errors in the test suite.

[   51s] =================================== FAILURES ===================================
[   51s] _______________________________ test_json_kwargs _______________________________
[   51s] 
[   51s] tmpdir = local('/tmp/pytest-of-abuild/pytest-0/test_json_kwargs0')
[   51s] 
[   51s]     def test_json_kwargs(tmpdir):
[   51s]         db_file = tmpdir.join('test.db')
[   51s] >       db = TinyDB(str(db_file), sort_keys=True, indent=4, separators=(',', ': '))
[   51s] 
[   51s] tests/test_storages.py:35: 
[   51s] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
[   51s] tinydb/database.py:165: in __init__
[   51s]     self._table = self.table(default_table)
[   51s] tinydb/database.py:196: in table
[   51s]     table = table_class(self._cls_storage_proxy(self._storage, name), name, **options)
[   51s] tinydb/database.py:302: in __init__
[   51s]     data = self._read()
[   51s] tinydb/database.py:409: in _read
[   51s]     return self._storage.read()
[   51s] tinydb/database.py:96: in read
[   51s]     self._storage.write(raw_data)
[   51s] _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
[   51s] 
[   51s] self = <tinydb.storages.JSONStorage object at 0x7f661e42b910>
[   51s] data = {'_default': {}}
[   51s] 
[   51s]     def write(self, data):
[   51s]         self._handle.seek(0)
[   51s] >       serialized = json.dumps(data, **self.kwargs)
[   51s] E       TypeError: 'separators' is an invalid keyword argument for this function
[   51s] 
[   51s] tinydb/storages.py:112: TypeError
[   51s] ___________________________ test_insert_invalid_dict ___________________________
[   51s] 
[   51s] tmpdir = local('/tmp/pytest-of-abuild/pytest-0/test_insert_invalid_dict0')
[   51s] 
[   51s]     def test_insert_invalid_dict(tmpdir):
[   51s]         path = str(tmpdir.join('db.json'))
[   51s]     
[   51s]         with TinyDB(path) as _db:
[   51s]             data = [{'int': 1}, {'int': 2}]
[   51s]             _db.insert_multiple(data)
[   51s]     
[   51s]             with pytest.raises(TypeError):
[   51s] >               _db.insert({'int': {'bark'}})  # Fails
[   51s] E               Failed: DID NOT RAISE <type 'exceptions.TypeError'>
[   51s] 

Builds at https://build.opensuse.org/package/show/home:jayvdb:py-new/python-tinydb

[msiemens]

I've now fixed this by skipping these tests when ujson is installed. Honestly, it seems like ujson is pretty much unmaintained, so I'm thinking about maybe not recommending its use any more. I've created an issue for discussing this at #263.

[jayvdb]

The problem has not been solved by https://github.com/msiemens/tinydb/commit/4e6f849e0dc698f468b53104f639f064dfd20691

If ujson is installed, using tinydb as documented breaks. i.e. use of separators etc in the constructor.

"Deprecating" it is not nice. The user isnt explicitly trying to use it with tinydb. They may need it for another part of their stack. It is merely available in their site-packages, and tinydb assumes it should use it, and fails.

[msiemens]

"Deprecating" it is not nice. The user isnt explicitly trying to use it with tinydb. They may need it for another part of their stack. It is merely available in their site-packages, and tinydb assumes it should use it, and fails.

You're right that I was a little quick to add a deprecation warning. I think it wasn't the best idea to add this auto-detection mechanism in the first place. Maybe it would be better to raise a warning that the behavior of JSONStorage may change if ujson is installed. We could also add an environment variable that disables automatic ujson usage so it's possible to run the test suite without errors even if ujson is installed.

In the long term I think we'd need to release a breaking release of TinyDB that removes ujson auto-import altogether.

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Feel free to reopen this if needed. Thank you for your contributions :heart:

[jayvdb]

Still an issue.

[msiemens]

This should be fixed with TinyDB v4 where ujson support has been dropped

[msiemens]

One still can write a custom storage that uses the ujson module but it won't be used automatically if found present

[jayvdb]

Thanks.