rtoml icon indicating copy to clipboard operation
rtoml copied to clipboard

Add argument `none_value` for `None` representation in loading and dumping

Open pwwang opened this issue 3 years ago • 8 comments

Related: https://github.com/samuelcolvin/rtoml/issues/23, https://github.com/toml-lang/toml/issues/30

  • load/loads
    # By default, nothing should be loaded as None, since it doesn't exist in TOML
    rtoml.loads('x = "null"') == {'x': "null"}
    rtoml.loads('x = ""') == {'x': ""}
    # Now with the PR, we can tell what to be interpreted as None
    rtoml.loads('x = "null"', none_value="null") == {'x': None}
    rtoml.loads('x = ""', none_value="") == {'x': None}
    
  • dump/dumps
    # By default, None dumps into "null"
    rtoml.dumps({"x": None}) == 'x = "null"\n'
    # Now with the PR, we can control what None should be dumped into
    rtoml.dumps({"x": None}, none_value="@None") == 'x = "@None"\n'
    

Reproducibility between loading and dumping using the same none_value:

s = 'x = "py:None"\n'
rtoml.loads(s, none_value="py:None") == {'x': None}
rtoml.dumps({'x': None}, none_value="py:None") == s

s = {'x': None}
rtoml.dumps(s, none_value="py:None") == 'x = "py:None"\n'
rtoml.loads('x = "py:None"\n', none_value="py:None") == s

The PR modifies the visitor at rust end, and no re-visit is needed to covert the values at python end (like suggested by #23). So it barely affects the speed.

I think the only "drawback" is that only string None-representation is supported. You can't convert 0 (integer) to None, for example, while loading, or vice versa while dumping.

You can also omit None values or items that are associated with None value with omit_none=True. See https://github.com/samuelcolvin/rtoml/issues/23#issuecomment-917640979

rtoml.dumps({'test': None}, omit_none=True) == ''
rtoml.dumps({'test': [1, None, 2]}, omit_none=True) == 'test = [1, 2]\n'
rtoml.dumps({'test': (1, None, 2)}, omit_none=True) == 'test = [1, 2]\n'
rtoml.dumps({'test': {'x': [None, {'y': [1, None, 2]}], 'z': None}}, omit_none=True)  # gives
"""
[[test.x]]
y = [1, 2]
"""
# But with omit_none=False
rtoml.dumps({'test': {'x': [None, {'y': [1, None, 2]}], 'z': None}}, omit_none=False, pretty=True)   # gives
"""
[test]
z = 'null'

[[test.x]]
y = [
    1,
    'null',
    2,
]

pwwang avatar Nov 12 '22 08:11 pwwang

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (1d0b60e) to head (5fc12d6).

:exclamation: Current head 5fc12d6 differs from pull request most recent head d6302af

Please upload reports for the commit d6302af to get more accurate results.

Additional details and impacted files
@@            Coverage Diff            @@
##              main       #53   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           29        29           
  Branches         5         5           
=========================================
  Hits            29        29           

codecov[bot] avatar Nov 12 '22 08:11 codecov[bot]

Looking good, suggestions:

  • Rename the argument to none_value
  • make none_value optional, default None when deserialising

samuelcolvin avatar Nov 12 '22 08:11 samuelcolvin

Revised.

pwwang avatar Nov 12 '22 08:11 pwwang

This looks awesome, I'll review properly when I'm at a computer.

samuelcolvin avatar Nov 12 '22 08:11 samuelcolvin

The omit_none argument was added at https://github.com/samuelcolvin/rtoml/pull/53/commits/3be85862ca75ae880ad307aceaaa5ed1c4763fce.

I know it's a sort of different functionality but it costs tiny modifications on the current code base. I can submit a separate PR if you want.

pwwang avatar Nov 12 '22 17:11 pwwang

As for the readme file, I think this None-value handling could be a selling point. Another point is that, in my opinion, you can state that rtoml is THE fastest TOML library for python, based on the benchmarking from https://github.com/pwwang/toml-bench

pwwang avatar Nov 13 '22 06:11 pwwang

As for the readme file, I think this None-value handling could be a selling point. Another point is that, in my opinion, you can state that rtoml is THE fastest TOML library for python, based on the benchmarking from https://github.com/pwwang/toml-bench

I agree on both, we should say "at the time of writing" just in case. Feel free to update here or in another PR.

samuelcolvin avatar Nov 13 '22 09:11 samuelcolvin

What happened to this PR?

caniko avatar May 06 '23 14:05 caniko

What happened to this PR?

I got very busy :smile:.

Fixing now.

samuelcolvin avatar Jun 24 '24 10:06 samuelcolvin

Thanks so much for this @pwwang.

samuelcolvin avatar Jun 24 '24 11:06 samuelcolvin

Awesome! I also mentioned this feature in https://github.com/pwwang/toml-bench

See sections:

  • https://github.com/pwwang/toml-bench/tree/master?tab=readme-ov-file#dumping-none-value
  • https://github.com/pwwang/toml-bench/tree/master?tab=readme-ov-file#dumping-key-none-pair
  • https://github.com/pwwang/toml-bench/tree/master?tab=readme-ov-file#dumping-list-with-none-value
  • https://github.com/pwwang/toml-bench/tree/master?tab=readme-ov-file#loading-none-like-values

pwwang avatar Jun 25 '24 19:06 pwwang