New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update database.py #104

Merged
merged 3 commits into from Jun 14, 2016

Conversation

3 participants
@KeksKeksKeks
Contributor

KeksKeksKeks commented Jun 13, 2016

Don't write db if _default is an empty Table.

If there are no documents in the _default table, TinyDB will write (and thus, re-serialize) the entire database.
This is because bool({}) == False, although the table does exist.

I have a JSON file under version control, which doesn't use the default table. Recently, I noticed that the file changes although I didn't change any document within the database.
Apparently, TinyDB.init reads the default table, which is empty in my case. An empty document evaluates to False in a boolean context, and TinyDB will write a new, empty table, effectively re-serializing the database on read.

KeksKeksKeks added some commits Jun 13, 2016

Update database.py
Don't write db if _default is an empty Table.

If there are no documents in the _default table, TinyDB will write (and thus, re-serialize) the entire database.
This is because bool({}) == False, although the table does exist.
Don't write the db on read.
This time, don't break the tests.
@KeksKeksKeks

This comment has been minimized.

Show comment
Hide comment
@KeksKeksKeks

KeksKeksKeks Jun 13, 2016

Contributor

Classical case of RTFM :(

I should have read the contribution instructions first. I have added a test to demonstrate the original behaviour, introduced changes to make it go away, and added the neccessary changes to make the coverage check happy.

Kudos for such a thorough code infrastructure! I enjoyed using (and now, contributing to) TinyDB.

Contributor

KeksKeksKeks commented Jun 13, 2016

Classical case of RTFM :(

I should have read the contribution instructions first. I have added a test to demonstrate the original behaviour, introduced changes to make it go away, and added the neccessary changes to make the coverage check happy.

Kudos for such a thorough code infrastructure! I enjoyed using (and now, contributing to) TinyDB.

@msiemens msiemens merged commit 331bb0d into msiemens:master Jun 14, 2016

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 99.742%
Details
@msiemens

This comment has been minimized.

Show comment
Hide comment
@msiemens

msiemens Jun 14, 2016

Owner

That makes sense, thank you! Just out of curiosity: I find it interesting that the VCS tracks the file modification time. Which one are you using?

Owner

msiemens commented Jun 14, 2016

That makes sense, thank you! Just out of curiosity: I find it interesting that the VCS tracks the file modification time. Which one are you using?

@KeksKeksKeks

This comment has been minimized.

Show comment
Hide comment
@KeksKeksKeks

KeksKeksKeks Jun 14, 2016

Contributor

You're welcome!

I use git, which doesn't track file modification time. Instead, because python dicts are unordered, two JSON serialisations of the same database will look different. And a different file content is, of course, then detected by the VCS.

Contributor

KeksKeksKeks commented Jun 14, 2016

You're welcome!

I use git, which doesn't track file modification time. Instead, because python dicts are unordered, two JSON serialisations of the same database will look different. And a different file content is, of course, then detected by the VCS.

@msiemens

This comment has been minimized.

Show comment
Hide comment
@msiemens

msiemens Jun 14, 2016

Owner

Ah, I see! Unexpected reordering of Python's dicts have bitten me more than once 😄

Owner

msiemens commented Jun 14, 2016

Ah, I see! Unexpected reordering of Python's dicts have bitten me more than once 😄

@eugene-eeo

This comment has been minimized.

Show comment
Hide comment
@eugene-eeo

eugene-eeo Jun 19, 2016

Contributor

Turns out there is a sort_keys option in the json.dumpsfunction. However I think that'd degrade performance slightly, but anyways here it is: http://stackoverflow.com/questions/10844064/items-in-json-object-are-out-of-order-using-json-dumps

Contributor

eugene-eeo commented Jun 19, 2016

Turns out there is a sort_keys option in the json.dumpsfunction. However I think that'd degrade performance slightly, but anyways here it is: http://stackoverflow.com/questions/10844064/items-in-json-object-are-out-of-order-using-json-dumps

msiemens added a commit that referenced this pull request Sep 14, 2016

Fix v3.2.1 changelog
As mentioned in sixty-north/cosmic-ray#171, PR #104 wasn't listed
in the changelog for v3.2.1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment