search

davebshow / goblin

A Python 3.5 rewrite of the TinkerPop 3 OGM Goblin
Other
93 stars 21 forks source link

Added variable indicating availability of separators keyword argument… #49

Closed H-Plus-Time closed 7 years ago

H-Plus-Time commented 7 years ago

ujson doesn't have the separators keyword argument (and it looks like the maintainer doesn't wish to add this...ever), so I've added a fairly lightweight flag in the try...except import statement to account for this, and a check around the json.dumps invocation (with or without separators, depending on the flag).

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 91.975% when pulling 3eca05a5e656458687277134c44bf3b3636a803a on H-Plus-Time:master into e792788f69dac01f08c12d0eda7dc420d3dbfee6 on davebshow:master.

davebshow commented 7 years ago

Hi @H-Plus-Time thanks for noticing this. I think that it makes more sense in general to just remove the separators kwarg as stated in the inline.

H-Plus-Time commented 7 years ago

Agreed, that's a better option. I assumed it was for eking out a little more compactness, though the effect diminishes with the length of keys and values.

coveralls commented 7 years ago

Coverage Status

Coverage remained the same at 91.975% when pulling faba818344ef4a22e48780ad4f9f847310df9ba1 on H-Plus-Time:master into e792788f69dac01f08c12d0eda7dc420d3dbfee6 on davebshow:master.

davebshow commented 7 years ago

Thanks @H-Plus-Time I'll go ahead an merge this.