bobfang1992 / pytomlpp

A python wrapper for tomlplusplus
https://bobfang1992.github.io/pytomlpp/pytomlpp.html
MIT License
86 stars 11 forks source link

Keep dict insertion order during `dump` #48

Closed h-vetinari closed 2 years ago

h-vetinari commented 3 years ago

Hey, thanks for this library!

I just followed down the rabbit-hole of figuring out what would be necessary to maintain insertion order, and basically, this is blocked on https://github.com/marzer/tomlplusplus/issues/28 / https://github.com/marzer/tomlplusplus/issues/62.

I just thought I'd open an issue here for someone else who might be wondering about this (and yes, I'm keen to see this resolved, even if it means fixing it on the tomlplusplus side). 🙃

marzer commented 3 years ago

fixing it on the tomlplusplus side

Fixing it implies that it's broken. It's not. It disregards order on purpose, as I justify quite comprehensively in the thread you linked. If you want this 'fixed', fix it in python, as toml++ will stay the way it is for the forseeable future.

bobfang1992 commented 3 years ago

I'd rather not to change tomlplusplus's behaviour unless there is a compelling reason. Could you give me some reason? Why this is needed?

bobfang1992 commented 3 years ago

Or can it be done with some params to control this behaviour instead of by default?

marzer commented 3 years ago

@bobfang1992 there's no way to do this C++-side without nontrivial modification to TOML++. The root of the issue is that the lib uses std::map to take advantage of C++17's heterogeneous lookup, a side effect being that std::map doesn't track insertion order, and iteration is simply ordered according to key, so things get re-ordered alphabetically.

Theoretically, If I made the underlying map configurable, a user could swap it out for something that maintained insertion order, but it would also need to support heterogeneous lookup for it to compile. To my knowledge nothing like that exists (though it has been a while since I looked, to be honest).

h-vetinari commented 3 years ago

@bobfang1992: I'd rather not to change tomlplusplus's behaviour unless there is a compelling reason. Could you give me some reason? Why this is needed?

Not least anytime you want to read & resave files that are not only machine processed - the machine doesn't care about the order, but since this is a config format that's supposed to be human-readable, a great many use-cases actually require that files are modifiable by humans (and order of entries for real-world things are semantically relevant beyond what the format spec or std::map require, so humans care about not having their order destroyed in the process).

I realized from looking at the code that this isn't possible without modifying toml++, which is why I commented there - I would suggest to focus the discussion of that in https://github.com/marzer/tomlplusplus/issues/28; I only opened the issue because other people who are discovering this python-wrapper will also stumble over the same problem.

bobfang1992 commented 3 years ago

cool thanks for the clarification. I will keep this open so people can easily spot this issue if they wanted the same thing. Seems nothing can be done on pytomlpp's side.

h-vetinari commented 3 years ago

Seems nothing can be done on pytomlpp's side.

It's not completely impossible (as hardly anything is...) - py_dict_to_toml_table could keep track of insertion order and save that information in a way that it can get reused when writing, but IMO it would be much more desirable to implement this on the toml++ side.

bobfang1992 commented 2 years ago

closing on this as we do not plan to support it.

pwwang commented 2 years ago

Here is an reference: https://github.com/pwwang/toml-bench#testdumpkeysorder even though it's not in the plan

zhihanyue commented 1 year ago

I think preserving order is crucial for generating config file. Because the config file is for humans. For example, we want to put some important settings (such as "general", "basic") at the top of the config file. How to implement this?

bobfang1992 commented 1 year ago

@yuezhihan Well in general in open source world there is a saying "be the change you want". I'd encourage you to take a look at the source code of this repo first, and see if we can do it here. It is a relatively simple code base and we might be able to do it here without touching the underlying tomlpp library.

h-vetinari commented 1 year ago

I'd encourage you to take a look at the source code of this repo first, and see if we can do it here.

I did take a look at the time, and I don't think it's possible without either: