samuelcolvin / rtoml

A fast TOML library for python implemented in rust.

https://pypi.org/project/rtoml/

MIT License

324 stars 28 forks source link

Preserve order #19

Closed manfredlotz closed 3 years ago

manfredlotz commented 3 years ago

I just did the following changes

added feature preserve_order to toml crate in Cargo.toml
added two tests in tests/test_order.py

codecov[bot] commented 3 years ago

Codecov Report

Merging #19 (8882a78) into master (0779d2c) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##            master       #19   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           44        44           
  Branches         7         7           
=========================================
  Hits            44        44

manfredlotz commented 3 years ago

Sorry, forgot to run make lint

samuelcolvin commented 3 years ago

You can reopen and push more commits to this pull request.

samuelcolvin commented 3 years ago

looking good, as per my comment above, would you be able to add test/tests for dumping as well as loading?

manfredlotz commented 3 years ago

looking good, as per my comment above, would you be able to add test/tests for dumping as well as loading?

To be honest, it is not quite clear to me what to test here...

samuelcolvin commented 3 years ago

Well that the order of a dict is preserved when calling rtoml.dump()

manfredlotz commented 3 years ago

When using rtoml.dump() I have to compare to a string which as you (rightly) pointed out earlier is not really nice.

samuelcolvin commented 3 years ago

Well, it's just the inverse of the current test: here, the input is a dict and the output is a string. In this case the tests are actually more elegant as you don't have the case of comparing dicts.

manfredlotz commented 3 years ago

Something like this?

def test_dumps_order(input_toml):
    loaded = rtoml.load(input_toml)

    assert input_toml == rtoml.dumps(loaded)

samuelcolvin commented 3 years ago

no, just:

def test_dumps_order(py_object, expected_toml):
    assert rtoml.dumps(py_object) == expected_toml

samuelcolvin commented 3 years ago

Thanks so much for this.

I tweaked the tests a little. I'll make a new release now.

samuelcolvin commented 3 years ago

I'll make a new release now.

Done.

manfredlotz commented 3 years ago

Thanks a lot!